Fix misinterpretation of maxAttempts (#497)

Author: igorbernstein2

gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallableFactory.java

@@ -74,10 +74,7 @@ public static <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> createBas
     }
     callable = new GrpcExceptionCallable<>(callable, callSettings.getRetryableCodes());
 
-    if (!callSettings.getRetryableCodes().isEmpty()
-        && callSettings.getRetrySettings().getMaxAttempts() > 1) {
-      callable = Callables.retrying(callable, callSettings, clientContext);
-    }
+    callable = Callables.retrying(callable, callSettings, clientContext);
 
     return callable;
   }

gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallableFactoryTest.java

@@ -32,6 +32,7 @@
 import com.google.api.gax.grpc.testing.FakeServiceGrpc;
 import com.google.api.gax.grpc.testing.FakeServiceImpl;
 import com.google.api.gax.grpc.testing.InProcessServer;
+import com.google.api.gax.retrying.RetrySettings;
 import com.google.api.gax.rpc.ClientContext;
 import com.google.api.gax.rpc.InvalidArgumentException;
 import com.google.api.gax.rpc.ServerStreamingCallSettings;
@@ -48,6 +49,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
+import org.threeten.bp.Duration;
 
 @RunWith(JUnit4.class)
 public class GrpcCallableFactoryTest {
@@ -84,7 +86,10 @@ public void createServerStreamingCallableRetryableExceptions() throws Exception
 
     // Base case: without config, invalid argument errors are not retryable.
     ServerStreamingCallSettings<Color, Money> nonRetryableSettings =
-        ServerStreamingCallSettings.<Color, Money>newBuilder().build();
+        ServerStreamingCallSettings.<Color, Money>newBuilder()
+            .setRetrySettings(
+                RetrySettings.newBuilder().setTotalTimeout(Duration.ofSeconds(1)).build())
+            .build();
 
     ServerStreamingCallable<Color, Money> nonRetryableCallable =
         GrpcCallableFactory.createServerStreamingCallable(
@@ -105,6 +110,8 @@ public void createServerStreamingCallableRetryableExceptions() throws Exception
     ServerStreamingCallSettings<Color, Money> retryableSettings =
         ServerStreamingCallSettings.<Color, Money>newBuilder()
             .setRetryableCodes(Code.INVALID_ARGUMENT)
+            .setRetrySettings(
+                RetrySettings.newBuilder().setTotalTimeout(Duration.ofSeconds(1)).build())
             .build();
 
     ServerStreamingCallable<Color, Money> retryableCallable =

gax/src/main/java/com/google/api/gax/rpc/Callables.java

@@ -55,6 +55,10 @@ public static <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> retrying(
       UnaryCallSettings<?, ?> callSettings,
       ClientContext clientContext) {
 
+    if (callSettings.getRetryableCodes().isEmpty()) {
+      return innerCallable;
+    }
+
     RetryAlgorithm<ResponseT> retryAlgorithm =
         new RetryAlgorithm<>(
             new ApiResultRetryAlgorithm<ResponseT>(),
@@ -72,9 +76,7 @@ public static <RequestT, ResponseT> ServerStreamingCallable<RequestT, ResponseT>
       ServerStreamingCallSettings<RequestT, ResponseT> callSettings,
       ClientContext clientContext) {
 
-    if (callSettings.getRetryableCodes().isEmpty()
-        || callSettings.getRetrySettings().getMaxAttempts() <= 1) {
-
+    if (callSettings.getRetryableCodes().isEmpty()) {
       return innerCallable;
     }
 

gax/src/test/java/com/google/api/gax/rpc/RetryingTest.java

@@ -34,6 +34,7 @@
 import com.google.api.gax.core.FakeApiClock;
 import com.google.api.gax.core.RecordingScheduler;
 import com.google.api.gax.retrying.RetrySettings;
+import com.google.api.gax.rpc.StatusCode.Code;
 import com.google.api.gax.rpc.testing.FakeCallContext;
 import com.google.api.gax.rpc.testing.FakeCallableFactory;
 import com.google.api.gax.rpc.testing.FakeChannel;
@@ -288,6 +289,7 @@ public void testUnknownStatusCode() {
 
   public static UnaryCallSettings<Integer, Integer> createSettings(RetrySettings retrySettings) {
     return UnaryCallSettings.<Integer, Integer>newUnaryCallSettingsBuilder()
+        .setRetryableCodes(Code.UNAVAILABLE)
         .setRetrySettings(retrySettings)
         .build();
   }