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

Author: tlrx

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;
@@ -148,15 +145,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
@@ -168,13 +161,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);
+                    }
                 );
             }
 

plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java

@@ -27,12 +27,28 @@
 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.repositories.gcs.GoogleCloudStorageService.InternalGoogleCloudStorageService;
 import org.elasticsearch.test.ESTestCase;
 
+import static java.util.Collections.emptyMap;
 import static org.hamcrest.Matchers.containsString;
+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 {
 
@@ -82,4 +98,45 @@ public void testClientCredential() throws Exception {
         InternalGoogleCloudStorageService service = new InternalGoogleCloudStorageService(env, credentials);
         assertSame(cred, service.getCredential("_default_", "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());
+    }
 }