[ggj] fix(engx): Handle nulls with poll() in MockServiceImpl (#693)

* fix(mixins): handle unit tests for mixin pagination methods * fix(engx): Handle nulls with poll() in MockServiceImpl
googleapis · Mar 13, 2021 · 3f531bd · 3f531bd
1 parent 69924c5
commit 3f531bd
Show file tree

Hide file tree

Showing 13 changed files with 247 additions and 223 deletions.
diff --git a/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/MockServiceImplClassComposer.java
@@ -28,9 +28,11 @@
 import com.google.api.generator.engine.ast.MethodDefinition;
 import com.google.api.generator.engine.ast.MethodInvocationExpr;
 import com.google.api.generator.engine.ast.NewObjectExpr;
+import com.google.api.generator.engine.ast.RelationalOperationExpr;
 import com.google.api.generator.engine.ast.ScopeNode;
 import com.google.api.generator.engine.ast.Statement;
 import com.google.api.generator.engine.ast.StringObjectValue;
+import com.google.api.generator.engine.ast.TernaryExpr;
 import com.google.api.generator.engine.ast.ThisObjectValue;
 import com.google.api.generator.engine.ast.TypeNode;
 import com.google.api.generator.engine.ast.ValueExpr;
@@ -304,7 +306,7 @@ private static MethodDefinition createGenericProtoMethodOverride(Method protoMet
                         .setVariableExpr(localResponseVarExpr.toBuilder().setIsDecl(true).build())
                         .setValueExpr(
                             MethodInvocationExpr.builder()
-                                .setMethodName("remove")
+                                .setMethodName("poll")
                                 .setExprReferenceExpr(responsesVarExpr)
                                 .setReturnType(objectType)
                                 .build())
@@ -485,15 +487,23 @@ private static Statement createHandleObjectStatement(
     }
 
     TypeNode exceptionType = TypeNode.withReference(ConcreteReference.withClazz(Exception.class));
+    // Constructs `response == null ? "" : response.getClass().getName()`.
     Expr actualResponseTypeString =
-        MethodInvocationExpr.builder()
-            .setExprReferenceExpr(
+        TernaryExpr.builder()
+            .setConditionExpr(
+                RelationalOperationExpr.equalToWithExprs(
+                    localResponseVarExpr, ValueExpr.createNullExpr()))
+            .setThenExpr(ValueExpr.withValue(StringObjectValue.withValue("null")))
+            .setElseExpr(
                 MethodInvocationExpr.builder()
-                    .setExprReferenceExpr(localResponseVarExpr)
-                    .setMethodName("getClass")
+                    .setExprReferenceExpr(
+                        MethodInvocationExpr.builder()
+                            .setExprReferenceExpr(localResponseVarExpr)
+                            .setMethodName("getClass")
+                            .build())
+                    .setMethodName("getName")
+                    .setReturnType(TypeNode.STRING)
                     .build())
-            .setMethodName("getName")
-            .setReturnType(TypeNode.STRING)
             .build();
     Function<TypeNode, Expr> typeToStrFn =
         t ->

diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/MockEchoImpl.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/MockEchoImpl.golden
@@ -45,7 +45,7 @@ public class MockEchoImpl extends EchoImplBase {
 
   @Override
   public void echo(EchoRequest request, StreamObserver<EchoResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof EchoResponse) {
       requests.add(request);
       responseObserver.onNext(((EchoResponse) response));
@@ -57,15 +57,15 @@ public class MockEchoImpl extends EchoImplBase {
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method Echo, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   EchoResponse.class.getName(),
                   Exception.class.getName())));
     }
   }
 
   @Override
   public void expand(ExpandRequest request, StreamObserver<EchoResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof EchoResponse) {
       requests.add(request);
       responseObserver.onNext(((EchoResponse) response));
@@ -77,7 +77,7 @@ public class MockEchoImpl extends EchoImplBase {
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method Expand, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   EchoResponse.class.getName(),
                   Exception.class.getName())));
     }
@@ -100,7 +100,7 @@ public class MockEchoImpl extends EchoImplBase {
                   new IllegalArgumentException(
                       String.format(
                           "Unrecognized response type %s for method Collect, expected %s or %s",
-                          response.getClass().getName(),
+                          response == null ? "null" : response.getClass().getName(),
                           EchoResponse.class.getName(),
                           Exception.class.getName())));
             }
@@ -136,7 +136,7 @@ public class MockEchoImpl extends EchoImplBase {
                   new IllegalArgumentException(
                       String.format(
                           "Unrecognized response type %s for method Chat, expected %s or %s",
-                          response.getClass().getName(),
+                          response == null ? "null" : response.getClass().getName(),
                           EchoResponse.class.getName(),
                           Exception.class.getName())));
             }
@@ -173,7 +173,7 @@ public class MockEchoImpl extends EchoImplBase {
                   new IllegalArgumentException(
                       String.format(
                           "Unrecognized response type %s for method ChatAgain, expected %s or %s",
-                          response.getClass().getName(),
+                          response == null ? "null" : response.getClass().getName(),
                           EchoResponse.class.getName(),
                           Exception.class.getName())));
             }
@@ -195,7 +195,7 @@ public class MockEchoImpl extends EchoImplBase {
   @Override
   public void pagedExpand(
       PagedExpandRequest request, StreamObserver<PagedExpandResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof PagedExpandResponse) {
       requests.add(request);
       responseObserver.onNext(((PagedExpandResponse) response));
@@ -207,7 +207,7 @@ public class MockEchoImpl extends EchoImplBase {
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method PagedExpand, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   PagedExpandResponse.class.getName(),
                   Exception.class.getName())));
     }
@@ -216,7 +216,7 @@ public class MockEchoImpl extends EchoImplBase {
   @Override
   public void simplePagedExpand(
       PagedExpandRequest request, StreamObserver<PagedExpandResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof PagedExpandResponse) {
       requests.add(request);
       responseObserver.onNext(((PagedExpandResponse) response));
@@ -228,15 +228,15 @@ public class MockEchoImpl extends EchoImplBase {
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method SimplePagedExpand, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   PagedExpandResponse.class.getName(),
                   Exception.class.getName())));
     }
   }
 
   @Override
   public void wait(WaitRequest request, StreamObserver<Operation> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof Operation) {
       requests.add(request);
       responseObserver.onNext(((Operation) response));
@@ -248,15 +248,15 @@ public class MockEchoImpl extends EchoImplBase {
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method Wait, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   Operation.class.getName(),
                   Exception.class.getName())));
     }
   }
 
   @Override
   public void block(BlockRequest request, StreamObserver<BlockResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof BlockResponse) {
       requests.add(request);
       responseObserver.onNext(((BlockResponse) response));
@@ -268,7 +268,7 @@ public class MockEchoImpl extends EchoImplBase {
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method Block, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   BlockResponse.class.getName(),
                   Exception.class.getName())));
     }

diff --git a/test/integration/goldens/asset/MockAssetServiceImpl.java b/test/integration/goldens/asset/MockAssetServiceImpl.java
@@ -63,7 +63,7 @@ public void reset() {
   @Override
   public void exportAssets(
       ExportAssetsRequest request, StreamObserver<Operation> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof Operation) {
       requests.add(request);
       responseObserver.onNext(((Operation) response));
@@ -75,7 +75,7 @@ public void exportAssets(
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method ExportAssets, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   Operation.class.getName(),
                   Exception.class.getName())));
     }
@@ -85,7 +85,7 @@ public void exportAssets(
   public void batchGetAssetsHistory(
       BatchGetAssetsHistoryRequest request,
       StreamObserver<BatchGetAssetsHistoryResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof BatchGetAssetsHistoryResponse) {
       requests.add(request);
       responseObserver.onNext(((BatchGetAssetsHistoryResponse) response));
@@ -97,15 +97,15 @@ public void batchGetAssetsHistory(
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method BatchGetAssetsHistory, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   BatchGetAssetsHistoryResponse.class.getName(),
                   Exception.class.getName())));
     }
   }
 
   @Override
   public void createFeed(CreateFeedRequest request, StreamObserver<Feed> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof Feed) {
       requests.add(request);
       responseObserver.onNext(((Feed) response));
@@ -117,13 +117,15 @@ public void createFeed(CreateFeedRequest request, StreamObserver<Feed> responseO
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method CreateFeed, expected %s or %s",
-                  response.getClass().getName(), Feed.class.getName(), Exception.class.getName())));
+                  response == null ? "null" : response.getClass().getName(),
+                  Feed.class.getName(),
+                  Exception.class.getName())));
     }
   }
 
   @Override
   public void getFeed(GetFeedRequest request, StreamObserver<Feed> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof Feed) {
       requests.add(request);
       responseObserver.onNext(((Feed) response));
@@ -135,14 +137,16 @@ public void getFeed(GetFeedRequest request, StreamObserver<Feed> responseObserve
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method GetFeed, expected %s or %s",
-                  response.getClass().getName(), Feed.class.getName(), Exception.class.getName())));
+                  response == null ? "null" : response.getClass().getName(),
+                  Feed.class.getName(),
+                  Exception.class.getName())));
     }
   }
 
   @Override
   public void listFeeds(
       ListFeedsRequest request, StreamObserver<ListFeedsResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof ListFeedsResponse) {
       requests.add(request);
       responseObserver.onNext(((ListFeedsResponse) response));
@@ -154,15 +158,15 @@ public void listFeeds(
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method ListFeeds, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   ListFeedsResponse.class.getName(),
                   Exception.class.getName())));
     }
   }
 
   @Override
   public void updateFeed(UpdateFeedRequest request, StreamObserver<Feed> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof Feed) {
       requests.add(request);
       responseObserver.onNext(((Feed) response));
@@ -174,13 +178,15 @@ public void updateFeed(UpdateFeedRequest request, StreamObserver<Feed> responseO
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method UpdateFeed, expected %s or %s",
-                  response.getClass().getName(), Feed.class.getName(), Exception.class.getName())));
+                  response == null ? "null" : response.getClass().getName(),
+                  Feed.class.getName(),
+                  Exception.class.getName())));
     }
   }
 
   @Override
   public void deleteFeed(DeleteFeedRequest request, StreamObserver<Empty> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof Empty) {
       requests.add(request);
       responseObserver.onNext(((Empty) response));
@@ -192,7 +198,7 @@ public void deleteFeed(DeleteFeedRequest request, StreamObserver<Empty> response
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method DeleteFeed, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   Empty.class.getName(),
                   Exception.class.getName())));
     }
@@ -202,7 +208,7 @@ public void deleteFeed(DeleteFeedRequest request, StreamObserver<Empty> response
   public void searchAllResources(
       SearchAllResourcesRequest request,
       StreamObserver<SearchAllResourcesResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof SearchAllResourcesResponse) {
       requests.add(request);
       responseObserver.onNext(((SearchAllResourcesResponse) response));
@@ -214,7 +220,7 @@ public void searchAllResources(
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method SearchAllResources, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   SearchAllResourcesResponse.class.getName(),
                   Exception.class.getName())));
     }
@@ -224,7 +230,7 @@ public void searchAllResources(
   public void searchAllIamPolicies(
       SearchAllIamPoliciesRequest request,
       StreamObserver<SearchAllIamPoliciesResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof SearchAllIamPoliciesResponse) {
       requests.add(request);
       responseObserver.onNext(((SearchAllIamPoliciesResponse) response));
@@ -236,7 +242,7 @@ public void searchAllIamPolicies(
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method SearchAllIamPolicies, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   SearchAllIamPoliciesResponse.class.getName(),
                   Exception.class.getName())));
     }
@@ -245,7 +251,7 @@ public void searchAllIamPolicies(
   @Override
   public void analyzeIamPolicy(
       AnalyzeIamPolicyRequest request, StreamObserver<AnalyzeIamPolicyResponse> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof AnalyzeIamPolicyResponse) {
       requests.add(request);
       responseObserver.onNext(((AnalyzeIamPolicyResponse) response));
@@ -257,7 +263,7 @@ public void analyzeIamPolicy(
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method AnalyzeIamPolicy, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   AnalyzeIamPolicyResponse.class.getName(),
                   Exception.class.getName())));
     }
@@ -266,7 +272,7 @@ public void analyzeIamPolicy(
   @Override
   public void analyzeIamPolicyLongrunning(
       AnalyzeIamPolicyLongrunningRequest request, StreamObserver<Operation> responseObserver) {
-    Object response = responses.remove();
+    Object response = responses.poll();
     if (response instanceof Operation) {
       requests.add(request);
       responseObserver.onNext(((Operation) response));
@@ -278,7 +284,7 @@ public void analyzeIamPolicyLongrunning(
           new IllegalArgumentException(
               String.format(
                   "Unrecognized response type %s for method AnalyzeIamPolicyLongrunning, expected %s or %s",
-                  response.getClass().getName(),
+                  response == null ? "null" : response.getClass().getName(),
                   Operation.class.getName(),
                   Exception.class.getName())));
     }