From cc814e57a0da56ed5b6011c62a88062a41bd0fe6 Mon Sep 17 00:00:00 2001 From: Luke Cwik Date: Wed, 26 Apr 2017 09:02:36 -0700 Subject: [PATCH] [BEAM-1871] Move RetryHttpRequestInitializer and remove deprecated methods --- sdks/java/extensions/gcp-core/pom.xml | 5 -- .../sdk/util/RetryHttpRequestInitializer.java | 60 +------------------ .../util/RetryHttpRequestInitializerTest.java | 11 +--- 3 files changed, 3 insertions(+), 73 deletions(-) rename sdks/java/{core => extensions/gcp-core}/src/main/java/org/apache/beam/sdk/util/RetryHttpRequestInitializer.java (75%) diff --git a/sdks/java/extensions/gcp-core/pom.xml b/sdks/java/extensions/gcp-core/pom.xml index 20bd62a45254..e9086a6b04a6 100644 --- a/sdks/java/extensions/gcp-core/pom.xml +++ b/sdks/java/extensions/gcp-core/pom.xml @@ -77,11 +77,6 @@ google-http-client-jackson2 - - com.google.oauth-client - google-oauth-client - - com.google.auth google-auth-library-oauth2-http diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/RetryHttpRequestInitializer.java b/sdks/java/extensions/gcp-core/src/main/java/org/apache/beam/sdk/util/RetryHttpRequestInitializer.java similarity index 75% rename from sdks/java/core/src/main/java/org/apache/beam/sdk/util/RetryHttpRequestInitializer.java rename to sdks/java/extensions/gcp-core/src/main/java/org/apache/beam/sdk/util/RetryHttpRequestInitializer.java index fa6e9136cd28..2b7135e94cd9 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/RetryHttpRequestInitializer.java +++ b/sdks/java/extensions/gcp-core/src/main/java/org/apache/beam/sdk/util/RetryHttpRequestInitializer.java @@ -42,10 +42,6 @@ * Implements a request initializer that adds retry handlers to all * HttpRequests. * - *

This allows chaining through to another HttpRequestInitializer, since - * clients have exactly one HttpRequestInitializer, and Credential is also - * a required HttpRequestInitializer. - * *

Also can take a HttpResponseInterceptor to be applied to the responses. */ public class RetryHttpRequestInitializer implements HttpRequestInitializer { @@ -121,9 +117,6 @@ public boolean handleResponse(HttpRequest request, HttpResponse response, } } - @Deprecated - private final HttpRequestInitializer chained; - private final HttpResponseInterceptor responseInterceptor; // response Interceptor to use private final NanoClock nanoClock; // used for testing @@ -136,17 +129,6 @@ public RetryHttpRequestInitializer() { this(Collections.emptyList()); } - /** - * @param chained a downstream HttpRequestInitializer, which will also be - * applied to HttpRequest initialization. May be null. - * - * @deprecated use {@link #RetryHttpRequestInitializer}. - */ - @Deprecated - public RetryHttpRequestInitializer(@Nullable HttpRequestInitializer chained) { - this(chained, Collections.emptyList()); - } - /** * @param additionalIgnoredResponseCodes a list of HTTP status codes that should not be logged. */ @@ -154,20 +136,6 @@ public RetryHttpRequestInitializer(Collection additionalIgnoredResponse this(additionalIgnoredResponseCodes, null); } - - /** - * @param chained a downstream HttpRequestInitializer, which will also be - * applied to HttpRequest initialization. May be null. - * @param additionalIgnoredResponseCodes a list of HTTP status codes that should not be logged. - * - * @deprecated use {@link #RetryHttpRequestInitializer(Collection)}. - */ - @Deprecated - public RetryHttpRequestInitializer(@Nullable HttpRequestInitializer chained, - Collection additionalIgnoredResponseCodes) { - this(chained, additionalIgnoredResponseCodes, null); - } - /** * @param additionalIgnoredResponseCodes a list of HTTP status codes that should not be logged. * @param responseInterceptor HttpResponseInterceptor to be applied on all requests. May be null. @@ -175,40 +143,20 @@ public RetryHttpRequestInitializer(@Nullable HttpRequestInitializer chained, public RetryHttpRequestInitializer( Collection additionalIgnoredResponseCodes, @Nullable HttpResponseInterceptor responseInterceptor) { - this(null, NanoClock.SYSTEM, Sleeper.DEFAULT, additionalIgnoredResponseCodes, - responseInterceptor); - } - - /** - * @param chained a downstream HttpRequestInitializer, which will also be applied to HttpRequest - * initialization. May be null. - * @param additionalIgnoredResponseCodes a list of HTTP status codes that should not be logged. - * @param responseInterceptor HttpResponseInterceptor to be applied on all requests. May be null. - * - * @deprecated use {@link #RetryHttpRequestInitializer(Collection, HttpResponseInterceptor)}. - */ - @Deprecated - public RetryHttpRequestInitializer( - @Nullable HttpRequestInitializer chained, - Collection additionalIgnoredResponseCodes, - @Nullable HttpResponseInterceptor responseInterceptor) { - this(chained, NanoClock.SYSTEM, Sleeper.DEFAULT, additionalIgnoredResponseCodes, + this(NanoClock.SYSTEM, Sleeper.DEFAULT, additionalIgnoredResponseCodes, responseInterceptor); } /** * Visible for testing. * - * @param chained a downstream HttpRequestInitializer, which will also be - * applied to HttpRequest initialization. May be null. * @param nanoClock used as a timing source for knowing how much time has elapsed. * @param sleeper used to sleep between retries. * @param additionalIgnoredResponseCodes a list of HTTP status codes that should not be logged. */ - RetryHttpRequestInitializer(@Nullable HttpRequestInitializer chained, + RetryHttpRequestInitializer( NanoClock nanoClock, Sleeper sleeper, Collection additionalIgnoredResponseCodes, HttpResponseInterceptor responseInterceptor) { - this.chained = chained; this.nanoClock = nanoClock; this.sleeper = sleeper; this.ignoredResponseCodes.addAll(additionalIgnoredResponseCodes); @@ -217,10 +165,6 @@ public RetryHttpRequestInitializer( @Override public void initialize(HttpRequest request) throws IOException { - if (chained != null) { - chained.initialize(request); - } - // Set a timeout for hanging-gets. // TODO: Do this exclusively for work requests. request.setReadTimeout(HANGING_GET_TIMEOUT_SEC * 1000); diff --git a/sdks/java/extensions/gcp-core/src/test/java/org/apache/beam/sdk/util/RetryHttpRequestInitializerTest.java b/sdks/java/extensions/gcp-core/src/test/java/org/apache/beam/sdk/util/RetryHttpRequestInitializerTest.java index 71554b573ac3..37551a4f9f0b 100644 --- a/sdks/java/extensions/gcp-core/src/test/java/org/apache/beam/sdk/util/RetryHttpRequestInitializerTest.java +++ b/sdks/java/extensions/gcp-core/src/test/java/org/apache/beam/sdk/util/RetryHttpRequestInitializerTest.java @@ -30,8 +30,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -import com.google.api.client.auth.oauth2.Credential; -import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpResponse; import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpResponseInterceptor; @@ -69,7 +67,6 @@ @RunWith(JUnit4.class) public class RetryHttpRequestInitializerTest { - @Mock private Credential mockCredential; @Mock private PrivateKey mockPrivateKey; @Mock private LowLevelHttpRequest mockLowLevelRequest; @Mock private LowLevelHttpResponse mockLowLevelResponse; @@ -106,7 +103,7 @@ protected LowLevelHttpRequest buildRequest(String method, String url) // only a single HttpRequestInitializer, and we use multiple Credential // types in the SDK, not all of which allow for retry configuration. RetryHttpRequestInitializer initializer = new RetryHttpRequestInitializer( - mockCredential, new MockNanoClock(), new Sleeper() { + new MockNanoClock(), new Sleeper() { @Override public void sleep(long millis) throws InterruptedException {} }, Arrays.asList(418 /* I'm a teapot */), mockHttpResponseInterceptor); @@ -118,7 +115,6 @@ public void sleep(long millis) throws InterruptedException {} public void tearDown() { verifyNoMoreInteractions(mockPrivateKey); verifyNoMoreInteractions(mockLowLevelRequest); - verifyNoMoreInteractions(mockCredential); verifyNoMoreInteractions(mockHttpResponseInterceptor); } @@ -133,7 +129,6 @@ public void testBasicOperation() throws IOException { HttpResponse response = result.executeUnparsed(); assertNotNull(response); - verify(mockCredential).initialize(any(HttpRequest.class)); verify(mockHttpResponseInterceptor).interceptResponse(any(HttpResponse.class)); verify(mockLowLevelRequest, atLeastOnce()) .addHeader(anyString(), anyString()); @@ -161,7 +156,6 @@ public void testErrorCodeForbidden() throws IOException { Assert.assertThat(e.getMessage(), Matchers.containsString("403")); } - verify(mockCredential).initialize(any(HttpRequest.class)); verify(mockHttpResponseInterceptor).interceptResponse(any(HttpResponse.class)); verify(mockLowLevelRequest, atLeastOnce()) .addHeader(anyString(), anyString()); @@ -188,7 +182,6 @@ public void testRetryableError() throws IOException { HttpResponse response = result.executeUnparsed(); assertNotNull(response); - verify(mockCredential).initialize(any(HttpRequest.class)); verify(mockHttpResponseInterceptor).interceptResponse(any(HttpResponse.class)); verify(mockLowLevelRequest, atLeastOnce()) .addHeader(anyString(), anyString()); @@ -212,7 +205,6 @@ public void testThrowIOException() throws IOException { HttpResponse response = result.executeUnparsed(); assertNotNull(response); - verify(mockCredential).initialize(any(HttpRequest.class)); verify(mockHttpResponseInterceptor).interceptResponse(any(HttpResponse.class)); verify(mockLowLevelRequest, atLeastOnce()) .addHeader(anyString(), anyString()); @@ -239,7 +231,6 @@ public Integer answer(InvocationOnMock invocation) { HttpResponse response = result.executeUnparsed(); assertNotNull(response); - verify(mockCredential).initialize(any(HttpRequest.class)); verify(mockHttpResponseInterceptor).interceptResponse(any(HttpResponse.class)); verify(mockLowLevelRequest, atLeastOnce()).addHeader(anyString(), anyString());