Merge pull request #1309 from garrettjonesgoogle/discogapic

merge from master
googleapis · May 30, 2017 · 41e4791 · 41e4791
2 parents eca6faa + b08fc02
commit 41e4791
Show file tree

Hide file tree

Showing 7 changed files with 127 additions and 40 deletions.
diff --git a/src/main/java/com/google/api/codegen/GapicContext.java b/src/main/java/com/google/api/codegen/GapicContext.java
@@ -117,6 +117,14 @@ protected boolean isSupported(Method method) {
     return !method.getRequestStreaming() && !method.getResponseStreaming();
   }
 
+  /**
+   * Returns true when the method supports retrying. By default, all supported methods support
+   * retrying. TODO: when isSupported is removed, this should return true.
+   */
+  public boolean isRetryingSupported(Method method) {
+    return isSupported(method);
+  }
+
   /** Returns a list of RPC methods supported by the context. */
   public List<Method> getSupportedMethods(Interface apiInterface) {
     List<Method> simples = new ArrayList<>(apiInterface.getMethods().size());

diff --git a/src/main/java/com/google/api/codegen/clientconfig/php/PhpClientConfigGapicContext.java b/src/main/java/com/google/api/codegen/clientconfig/php/PhpClientConfigGapicContext.java
@@ -0,0 +1,33 @@
+/* Copyright 2016 Google Inc
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.api.codegen.clientconfig.php;
+
+import com.google.api.codegen.clientconfig.ClientConfigGapicContext;
+import com.google.api.codegen.config.GapicProductConfig;
+import com.google.api.tools.framework.model.Method;
+import com.google.api.tools.framework.model.Model;
+
+public class PhpClientConfigGapicContext extends ClientConfigGapicContext {
+
+  /** Constructs the client config codegen context. */
+  public PhpClientConfigGapicContext(Model model, GapicProductConfig config) {
+    super(model, config);
+  }
+
+  @Override
+  public boolean isRetryingSupported(Method method) {
+    return isSupported(method) && !method.getRequestStreaming() && !method.getResponseStreaming();
+  }
+}
diff --git a/src/main/java/com/google/api/codegen/gapic/MainGapicProviderFactory.java b/src/main/java/com/google/api/codegen/gapic/MainGapicProviderFactory.java
@@ -19,6 +19,7 @@
 import com.google.api.codegen.SnippetSetRunner;
 import com.google.api.codegen.clientconfig.ClientConfigGapicContext;
 import com.google.api.codegen.clientconfig.ClientConfigSnippetSetRunner;
+import com.google.api.codegen.clientconfig.php.PhpClientConfigGapicContext;
 import com.google.api.codegen.config.GapicProductConfig;
 import com.google.api.codegen.config.PackageMetadataConfig;
 import com.google.api.codegen.nodejs.NodeJSCodePathMapper;
@@ -260,7 +261,7 @@ public static List<GapicProvider<? extends Object>> defaultCreate(
             CommonGapicProvider.<Interface>newBuilder()
                 .setModel(model)
                 .setView(new InterfaceView())
-                .setContext(new ClientConfigGapicContext(model, productConfig))
+                .setContext(new PhpClientConfigGapicContext(model, productConfig))
                 .setSnippetSetRunner(
                     new ClientConfigSnippetSetRunner<Interface>(
                         SnippetSetRunner.SNIPPET_RESOURCE_ROOT))

diff --git a/src/main/resources/com/google/api/codegen/clientconfig/json.snip b/src/main/resources/com/google/api/codegen/clientconfig/json.snip
@@ -88,12 +88,12 @@
                 retryCodesName = methodConfig.getRetryCodesConfigName, \
                 retryParamsName = methodConfig.getRetrySettingsConfigName
             "{@method.getSimpleName}": {
-              @if or(and(retryCodesName, retryParamsName), isBatching)
+              @if or(and(and(retryCodesName, retryParamsName), context.isRetryingSupported(method)), isBatching)
                 "timeout_millis": {@timeout},
               @else
                 "timeout_millis": {@timeout}
               @end
-              @if and(retryCodesName, retryParamsName)
+              @if and(and(retryCodesName, retryParamsName), context.isRetryingSupported(method))
                   "retry_codes_name": "{@retryCodesName}",
                   @if isBatching
                       "retry_params_name": "{@retryParamsName}",

diff --git a/src/main/resources/com/google/api/codegen/php/test.snip b/src/main/resources/com/google/api/codegen/php/test.snip
@@ -134,7 +134,7 @@
         $this->assertTrue($grpcStub->isExhausted());
 
         $status = new stdClass();
-        $status->code = Grpc\STATUS_INTERNAL;
+        $status->code = Grpc\STATUS_DATA_LOSS;
         $status->details = 'internal error';
         $grpcStub->addResponse(null, $status);
 
@@ -152,6 +152,8 @@
             $this->assertEquals($status->details, $ex->getMessage());
         }
 
+        // Call getReceivedCalls to ensure the stub is exhausted
+        $grpcStub->getReceivedCalls();
         $this->assertTrue($grpcStub->isExhausted());
     }
 @end
@@ -253,7 +255,7 @@
         $grpcStub->addResponse($incompleteOperation);
 
         $status = new stdClass();
-        $status->code = Grpc\STATUS_INTERNAL;
+        $status->code = Grpc\STATUS_DATA_LOSS;
         $status->details = 'internal error';
         $operationsStub->addResponse(null, $status);
 
@@ -278,6 +280,9 @@
             $this->assertEquals($status->details, $ex->getMessage());
         }
 
+        // Call getReceivedCalls to ensure the stubs are exhausted
+        $grpcStub->getReceivedCalls();
+        $operationsStub->getReceivedCalls();
         $this->assertTrue($grpcStub->isExhausted());
         $this->assertTrue($operationsStub->isExhausted());
     }

diff --git a/src/test/java/com/google/api/codegen/testdata/php_json_library.baseline b/src/test/java/com/google/api/codegen/testdata/php_json_library.baseline
@@ -119,24 +119,16 @@
           "retry_params_name": "default"
         },
         "StreamShelves": {
-          "timeout_millis": 30000,
-          "retry_codes_name": "idempotent",
-          "retry_params_name": "default"
+          "timeout_millis": 30000
         },
         "StreamBooks": {
-          "timeout_millis": 30000,
-          "retry_codes_name": "idempotent",
-          "retry_params_name": "default"
+          "timeout_millis": 30000
         },
         "DiscussBook": {
-          "timeout_millis": 30000,
-          "retry_codes_name": "non_idempotent",
-          "retry_params_name": "default"
+          "timeout_millis": 30000
         },
         "MonologAboutBook": {
-          "timeout_millis": 30000,
-          "retry_codes_name": "non_idempotent",
-          "retry_params_name": "default"
+          "timeout_millis": 30000
         },
         "FindRelatedBooks": {
           "timeout_millis": 3000,