Create new handlers for every new request in GoogleCloudStorageService (#27339)

This commit changes the DefaultHttpRequestInitializer in order to make
it create new HttpIOExceptionHandler and HttpUnsuccessfulResponseHandler
for every new HTTP request instead of reusing the same two handlers for
all requests.

Closes #27092
This commit is contained in:
Tanguy Leroux 2017-11-14 11:43:32 +01:00 committed by GitHub
parent 1caa5c8e32
commit dd51c231ac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 73 additions and 25 deletions

View File

@ -23,7 +23,6 @@ import com.google.api.client.googleapis.auth.oauth2.GoogleCredential;
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.Storage;
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;
@ -46,8 +43,6 @@ import org.elasticsearch.env.Environment;
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;
@ -76,16 +71,11 @@ interface GoogleCloudStorageService {
*/
class InternalGoogleCloudStorageService extends AbstractComponent implements GoogleCloudStorageService {
private static final String DEFAULT = "_default_";
private final Environment environment;
/** Credentials identified by client name. */
private final Map<String, GoogleCredential> credentials;
InternalGoogleCloudStorageService(Environment environment, Map<String, GoogleCredential> credentials) {
super(environment.settings());
this.environment = environment;
this.credentials = credentials;
}
@ -131,15 +121,11 @@ interface GoogleCloudStorageService {
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
@ -151,13 +137,14 @@ interface GoogleCloudStorageService {
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);
}
);
}

View File

@ -19,17 +19,35 @@
package org.elasticsearch.repositories.gcs;
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 com.google.api.client.googleapis.auth.oauth2.GoogleCredential;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
import org.elasticsearch.repositories.gcs.GoogleCloudStorageService.InternalGoogleCloudStorageService;
import org.elasticsearch.test.ESTestCase;
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 {
@ -47,13 +65,56 @@ public class GoogleCloudStorageServiceTests extends ESTestCase {
}
};
assertSame(cred, service.getCredential("default"));
service.new DefaultHttpRequestInitializer(cred, null, null);
}
public void testClientCredential() throws Exception {
GoogleCredential cred = GoogleCredential.fromStream(getDummyCredentialStream());
Map<String, GoogleCredential> credentials = Collections.singletonMap("clientname", cred);
Map<String, GoogleCredential> 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());
}
}