From 2c284dd0d8f9f7b97a86ceb032d85f50687eb55c Mon Sep 17 00:00:00 2001
From: JesseLovelace <43148100+JesseLovelace@users.noreply.github.com>
Date: Tue, 25 Sep 2018 10:11:18 -0700
Subject: [PATCH] Add retries to downloadEmulator() to mitigate transient
 network issues (#3719)

* Add retries to downloadEmulator() to mitigate transient network issues
---
 .../cloud/testing/BaseEmulatorHelper.java     | 14 ++++-
 .../cloud/testing/BaseEmulatorHelperTest.java | 56 +++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java
index 2f90f596467a..841d7e3e0716 100644
--- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java
+++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java
@@ -16,7 +16,10 @@
 
 package com.google.cloud.testing;
 
+import com.google.api.core.CurrentMillisClock;
 import com.google.api.core.InternalApi;
+import com.google.cloud.ExceptionHandler;
+import com.google.cloud.RetryHelper;
 import com.google.cloud.ServiceOptions;
 import com.google.common.io.CharStreams;
 import com.google.common.util.concurrent.SettableFuture;
@@ -46,6 +49,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -366,7 +370,15 @@ public boolean isAvailable() {
 
     @Override
     public void start() throws IOException {
-      Path emulatorPath = downloadEmulator();
+      ExceptionHandler retryOnAnythingExceptionHandler = ExceptionHandler.newBuilder().retryOn(Exception.class).build();
+
+      Path emulatorPath = RetryHelper.runWithRetries(new Callable<Path>() {
+          @Override
+          public Path call() throws IOException {
+              return downloadEmulator();
+          }
+        }, ServiceOptions.getDefaultRetrySettings(), retryOnAnythingExceptionHandler,
+              CurrentMillisClock.getDefaultClock());
       process = CommandWrapper.create()
           .setCommand(commandText)
           .setDirectory(emulatorPath)
diff --git a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java
index e8f5ed565fe4..7cb4a4ab078b 100644
--- a/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java
+++ b/google-cloud-clients/google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java
@@ -20,9 +20,15 @@
 import com.google.cloud.ServiceOptions;
 import com.google.cloud.testing.BaseEmulatorHelper.EmulatorRunner;
 import com.google.common.collect.ImmutableList;
+
 import java.io.ByteArrayInputStream;
+import java.io.EOFException;
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLStreamHandler;
 import java.util.List;
 import java.util.concurrent.TimeoutException;
 import java.util.logging.Logger;
@@ -96,6 +102,41 @@ public void testEmulatorHelper() throws IOException, InterruptedException, Timeo
     EasyMock.verify();
   }
 
+  @Test
+  public void testEmulatorHelperDownloadWithRetries() throws IOException, InterruptedException, TimeoutException {
+    String mockExternalForm = "mockExternalForm";
+    String mockInputStream = "mockInputStream";
+    String mockProtocol = "mockProtocol";
+    String mockFile = "mockFile";
+    String mockCommandText = "mockCommandText";
+
+    MockURLStreamHandler mockURLStreamHandler = EasyMock.createMock(MockURLStreamHandler.class);
+    URLConnection mockURLConnection = EasyMock.mock(URLConnection.class);
+
+    EasyMock.expect(mockURLStreamHandler.toExternalForm(EasyMock.anyObject(URL.class)))
+            .andReturn(mockExternalForm).anyTimes();
+    EasyMock.expect(mockURLConnection.getInputStream())
+            .andReturn(new ByteArrayInputStream(mockInputStream.getBytes())).anyTimes();
+    EasyMock.expect(mockURLStreamHandler.openConnection(EasyMock.anyObject(URL.class)))
+            .andThrow(new EOFException()).times(1);
+    EasyMock.expect(mockURLStreamHandler.openConnection(EasyMock.anyObject(URL.class)))
+            .andReturn(mockURLConnection).times(1);
+    EasyMock.replay(mockURLStreamHandler, mockURLConnection);
+
+    URL url = new URL(mockProtocol, null, 0, mockFile, mockURLStreamHandler);
+    BaseEmulatorHelper.DownloadableEmulatorRunner runner =
+            new BaseEmulatorHelper.DownloadableEmulatorRunner(ImmutableList.of(mockCommandText), url, null);
+
+    File cachedFile = new File(System.getProperty("java.io.tmpdir"), mockExternalForm);
+    cachedFile.delete(); //Clear the cached version so we're always testing the download
+
+    runner.start();
+
+    EasyMock.verify();
+
+    cachedFile.delete(); //Cleanup
+  }
+
   @Test
   public void testEmulatorHelperMultipleRunners() throws IOException, InterruptedException, TimeoutException {
     Process process = EasyMock.createStrictMock(Process.class);
@@ -117,4 +158,19 @@ public void testEmulatorHelperMultipleRunners() throws IOException, InterruptedE
     helper.stop(Duration.ofMinutes(1));
     EasyMock.verify();
   }
+
+  /**
+   * URLStreamHandler has a protected method which needs to be mocked, so we need our own implementation in this package
+   */
+  private class MockURLStreamHandler extends URLStreamHandler {
+    @Override
+    protected URLConnection openConnection(URL u) throws IOException {
+      return null;
+    }
+
+    @Override
+    protected String toExternalForm(URL u) {
+      return null;
+    }
+  }
 }