diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java index 50fd071accf48..91cc5c0fccdec 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java @@ -23,7 +23,6 @@ import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; import com.google.api.client.http.HttpBackOffIOExceptionHandler; import com.google.api.client.http.HttpBackOffUnsuccessfulResponseHandler; -import com.google.api.client.http.HttpIOExceptionHandler; import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpRequestInitializer; import com.google.api.client.http.HttpUnsuccessfulResponseHandler; @@ -34,9 +33,7 @@ import com.google.api.services.storage.StorageScopes; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.component.AbstractComponent; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.SecureSetting; -import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -45,8 +42,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -77,16 +72,11 @@ Storage createClient(String clientName, String application, */ class InternalGoogleCloudStorageService extends AbstractComponent implements GoogleCloudStorageService { - private static final String DEFAULT = "_default_"; - - private final Environment environment; - /** Credentials identified by client name. */ private final Map credentials; InternalGoogleCloudStorageService(Environment environment, Map credentials) { super(environment.settings()); - this.environment = environment; this.credentials = credentials; } @@ -132,15 +122,11 @@ class DefaultHttpRequestInitializer implements HttpRequestInitializer { private final TimeValue connectTimeout; private final TimeValue readTimeout; private final GoogleCredential credential; - private final HttpUnsuccessfulResponseHandler handler; - private final HttpIOExceptionHandler ioHandler; DefaultHttpRequestInitializer(GoogleCredential credential, TimeValue connectTimeout, TimeValue readTimeout) { this.credential = credential; this.connectTimeout = connectTimeout; this.readTimeout = readTimeout; - this.handler = new HttpBackOffUnsuccessfulResponseHandler(newBackOff()); - this.ioHandler = new HttpBackOffIOExceptionHandler(newBackOff()); } @Override @@ -152,13 +138,14 @@ public void initialize(HttpRequest request) throws IOException { request.setReadTimeout((int) readTimeout.millis()); } - request.setIOExceptionHandler(ioHandler); + request.setIOExceptionHandler(new HttpBackOffIOExceptionHandler(newBackOff())); request.setInterceptor(credential); + final HttpUnsuccessfulResponseHandler handler = new HttpBackOffUnsuccessfulResponseHandler(newBackOff()); request.setUnsuccessfulResponseHandler((req, resp, supportsRetry) -> { - // Let the credential handle the response. If it failed, we rely on our backoff handler - return credential.handleResponse(req, resp, supportsRetry) || handler.handleResponse(req, resp, supportsRetry); - } + // Let the credential handle the response. If it failed, we rely on our backoff handler + return credential.handleResponse(req, resp, supportsRetry) || handler.handleResponse(req, resp, supportsRetry); + } ); } diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java index a12cd4fdb5c94..07bd6974c6513 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java @@ -19,18 +19,36 @@ package org.elasticsearch.repositories.gcs; -import java.io.IOException; -import java.io.InputStream; -import java.util.Collections; -import java.util.Map; - import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; +import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpIOExceptionHandler; +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpRequestFactory; +import com.google.api.client.http.HttpRequestInitializer; +import com.google.api.client.http.HttpResponse; +import com.google.api.client.http.HttpUnsuccessfulResponseHandler; +import com.google.api.client.testing.http.MockHttpTransport; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.repositories.gcs.GoogleCloudStorageService.InternalGoogleCloudStorageService; import org.elasticsearch.test.ESTestCase; +import java.io.IOException; +import java.io.InputStream; +import java.util.Collections; +import java.util.Map; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + public class GoogleCloudStorageServiceTests extends ESTestCase { private InputStream getDummyCredentialStream() throws IOException { @@ -47,13 +65,56 @@ GoogleCredential getDefaultCredential() throws IOException { } }; assertSame(cred, service.getCredential("default")); + + service.new DefaultHttpRequestInitializer(cred, null, null); } public void testClientCredential() throws Exception { GoogleCredential cred = GoogleCredential.fromStream(getDummyCredentialStream()); - Map credentials = Collections.singletonMap("clientname", cred); + Map credentials = singletonMap("clientname", cred); Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build()); InternalGoogleCloudStorageService service = new InternalGoogleCloudStorageService(env, credentials); assertSame(cred, service.getCredential("clientname")); } + + /** + * Test that the {@link InternalGoogleCloudStorageService.DefaultHttpRequestInitializer} attaches new instances + * of {@link HttpIOExceptionHandler} and {@link HttpUnsuccessfulResponseHandler} for every HTTP requests. + */ + public void testDefaultHttpRequestInitializer() throws IOException { + final Environment environment = mock(Environment.class); + when(environment.settings()).thenReturn(Settings.EMPTY); + + final GoogleCredential credential = mock(GoogleCredential.class); + when(credential.handleResponse(any(HttpRequest.class), any(HttpResponse.class), anyBoolean())).thenReturn(false); + + final TimeValue readTimeout = TimeValue.timeValueSeconds(randomIntBetween(1, 120)); + final TimeValue connectTimeout = TimeValue.timeValueSeconds(randomIntBetween(1, 120)); + + final InternalGoogleCloudStorageService service = new InternalGoogleCloudStorageService(environment, emptyMap()); + final HttpRequestInitializer initializer = service.new DefaultHttpRequestInitializer(credential, connectTimeout, readTimeout); + final HttpRequestFactory requestFactory = new MockHttpTransport().createRequestFactory(initializer); + + final HttpRequest request1 = requestFactory.buildGetRequest(new GenericUrl()); + assertEquals((int) connectTimeout.millis(), request1.getConnectTimeout()); + assertEquals((int) readTimeout.millis(), request1.getReadTimeout()); + assertSame(credential, request1.getInterceptor()); + assertNotNull(request1.getIOExceptionHandler()); + assertNotNull(request1.getUnsuccessfulResponseHandler()); + + final HttpRequest request2 = requestFactory.buildGetRequest(new GenericUrl()); + assertEquals((int) connectTimeout.millis(), request2.getConnectTimeout()); + assertEquals((int) readTimeout.millis(), request2.getReadTimeout()); + assertSame(request1.getInterceptor(), request2.getInterceptor()); + assertNotNull(request2.getIOExceptionHandler()); + assertNotSame(request1.getIOExceptionHandler(), request2.getIOExceptionHandler()); + assertNotNull(request2.getUnsuccessfulResponseHandler()); + assertNotSame(request1.getUnsuccessfulResponseHandler(), request2.getUnsuccessfulResponseHandler()); + + request1.getUnsuccessfulResponseHandler().handleResponse(null, null, false); + verify(credential, times(1)).handleResponse(any(HttpRequest.class), any(HttpResponse.class), anyBoolean()); + + request2.getUnsuccessfulResponseHandler().handleResponse(null, null, false); + verify(credential, times(2)).handleResponse(any(HttpRequest.class), any(HttpResponse.class), anyBoolean()); + } }