Revert "fix: Get numeric value for Enum fields if it is configured as query param or path param (#1042)"

This reverts commit 4b5eadb.

Author: emmileaf

src/main/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposer.java

@@ -59,7 +59,6 @@
 import com.google.api.generator.gapic.model.OperationResponse;
 import com.google.api.generator.gapic.model.Service;
 import com.google.api.generator.gapic.utils.JavaStyle;
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.BiMap;
 import com.google.common.collect.ImmutableList;
 import com.google.protobuf.TypeRegistry;
@@ -75,7 +74,6 @@
 import java.util.stream.Collectors;
 
 public class HttpJsonServiceStubClassComposer extends AbstractTransportServiceStubClassComposer {
-
   private static final HttpJsonServiceStubClassComposer INSTANCE =
       new HttpJsonServiceStubClassComposer();
 
@@ -942,11 +940,9 @@ private Expr createFieldsExtractorClassInstance(
       for (int i = 0; i < descendantFields.length; i++) {
         String currFieldName = descendantFields[i];
         String bindingFieldMethodName =
-            getBindingFieldMethodName(
-                httpBindingFieldName,
-                descendantFields.length,
-                i,
-                JavaStyle.toUpperCamelCase(currFieldName));
+            (i < descendantFields.length - 1 || !httpBindingFieldName.isRepeated())
+                ? String.format("get%s", JavaStyle.toUpperCamelCase(currFieldName))
+                : String.format("get%sList", JavaStyle.toUpperCamelCase(currFieldName));
         requestFieldGetterExprBuilder =
             requestFieldGetterExprBuilder.setMethodName(bindingFieldMethodName);
 
@@ -1001,7 +997,6 @@ private Expr createFieldsExtractorClassInstance(
       }
     }
 
-    // Add a fixed query param for numeric enum, see b/232457244 for details
     if (restNumericEnumsEnabled && serializerMethodName.equals("putQueryParam")) {
       ImmutableList.Builder<Expr> paramsPutArgs = ImmutableList.builder();
 
@@ -1028,20 +1023,6 @@ private Expr createFieldsExtractorClassInstance(
         .build();
   }
 
-  @VisibleForTesting
-  String getBindingFieldMethodName(
-      HttpBinding httpBindingField, int descendantFieldsLengths, int index, String currFieldName) {
-    if (index == descendantFieldsLengths - 1) {
-      if (httpBindingField.isRepeated()) {
-        return String.format("get%sList", currFieldName);
-      }
-      if (httpBindingField.isEnum()) {
-        return String.format("get%sValue", currFieldName);
-      }
-    }
-    return String.format("get%s", currFieldName);
-  }
-
   private List<Expr> getHttpMethodTypeExpr(Method protoMethod) {
     return Collections.singletonList(
         ValueExpr.withValue(

src/main/java/com/google/api/generator/gapic/model/HttpBindings.java

@@ -36,54 +36,21 @@ public enum HttpVerb {
 
   @AutoValue
   public abstract static class HttpBinding implements Comparable<HttpBinding> {
-
-    // The fully qualified name of the field. e.g. request.complex_object.another_object.name
     public abstract String name();
 
     abstract String lowerCamelName();
 
-    // An object that contains all info of the leaf level field
-    @Nullable
-    public abstract Field field();
+    public abstract boolean isOptional();
 
-    public boolean isOptional() {
-      return field() != null && field().isProto3Optional();
-    }
-
-    public boolean isRepeated() {
-      return field() != null && field().isRepeated();
-    }
-
-    public boolean isEnum() {
-      return field() != null && field().isEnum();
-    }
+    public abstract boolean isRepeated();
 
     @Nullable
     public abstract String valuePattern();
 
-    public static HttpBindings.HttpBinding.Builder builder() {
-      return new AutoValue_HttpBindings_HttpBinding.Builder();
-    }
-
-    @AutoValue.Builder
-    public abstract static class Builder {
-
-      public abstract HttpBindings.HttpBinding.Builder setName(String name);
-
-      public abstract HttpBindings.HttpBinding.Builder setField(Field field);
-
-      abstract HttpBindings.HttpBinding.Builder setLowerCamelName(String lowerCamelName);
-
-      public abstract HttpBindings.HttpBinding.Builder setValuePattern(String valuePattern);
-
-      abstract String name();
-
-      abstract HttpBindings.HttpBinding autoBuild();
-
-      public HttpBindings.HttpBinding build() {
-        setLowerCamelName(JavaStyle.toLowerCamelCase(name()));
-        return autoBuild();
-      }
+    public static HttpBinding create(
+        String name, boolean isOptional, boolean isRepeated, String valuePattern) {
+      return new AutoValue_HttpBindings_HttpBinding(
+          name, JavaStyle.toLowerCamelCase(name), isOptional, isRepeated, valuePattern);
     }
 
     // Do not forget to keep it in sync with equals() implementation.

src/main/java/com/google/api/generator/gapic/protoparser/HttpRuleParser.java

@@ -82,11 +82,6 @@ private static HttpBindings parseHttpRuleHelper(
     Map<String, String> patternSampleValues = constructPathValuePatterns(pattern);
 
     // TODO: support nested message fields bindings
-    // Nested message fields bindings for query params are already supported as part of
-    // https://github.com/googleapis/gax-java/pull/1784,
-    // however we need to excludes fields that are already configured for path params and body, see
-    // https://github.com/googleapis/googleapis/blob/532289228eaebe77c42438f74b8a5afa85fee1b6/google/api/http.proto#L208 for details,
-    // the current logic does not exclude fields that are more than one level deep.
     String body = httpRule.getBody();
     Set<String> bodyParamNames;
     Set<String> queryParamNames;
@@ -138,9 +133,8 @@ private static Set<HttpBinding> validateAndConstructHttpBindings(
       String patternSampleValue =
           patternSampleValues != null ? patternSampleValues.get(paramName) : null;
       String[] subFields = paramName.split("\\.");
-      HttpBinding.Builder httpBindingBuilder = HttpBinding.builder().setName(paramName);
       if (inputMessage == null) {
-        httpBindings.add(httpBindingBuilder.setValuePattern(patternSampleValue).build());
+        httpBindings.add(HttpBinding.create(paramName, false, false, patternSampleValue));
         continue;
       }
       Message nestedMessage = inputMessage;
@@ -162,7 +156,8 @@ private static Set<HttpBinding> validateAndConstructHttpBindings(
           }
           Field field = nestedMessage.fieldMap().get(subFieldName);
           httpBindings.add(
-              httpBindingBuilder.setValuePattern(patternSampleValue).setField(field).build());
+              HttpBinding.create(
+                  paramName, field.isProto3Optional(), field.isRepeated(), patternSampleValue));
         }
       }
     }

src/test/java/com/google/api/generator/gapic/composer/rest/HttpJsonServiceStubClassComposerTest.java

@@ -14,35 +14,23 @@
 
 package com.google.api.generator.gapic.composer.rest;
 
-import com.google.api.generator.engine.ast.TypeNode;
 import com.google.api.generator.engine.writer.JavaWriterVisitor;
-import com.google.api.generator.gapic.model.Field;
 import com.google.api.generator.gapic.model.GapicClass;
 import com.google.api.generator.gapic.model.GapicContext;
-import com.google.api.generator.gapic.model.HttpBindings.HttpBinding;
 import com.google.api.generator.gapic.model.Service;
 import com.google.api.generator.test.framework.Assert;
 import com.google.api.generator.test.framework.Utils;
-import com.google.common.truth.Truth;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import org.junit.Before;
 import org.junit.Test;
 
 public class HttpJsonServiceStubClassComposerTest {
-
-  private HttpJsonServiceStubClassComposer composer;
-
-  @Before
-  public void setUp() throws Exception {
-    composer = HttpJsonServiceStubClassComposer.instance();
-  }
-
   @Test
   public void generateServiceClasses() {
     GapicContext context = RestTestProtoLoader.instance().parseCompliance();
     Service echoProtoService = context.services().get(0);
-    GapicClass clazz = composer.generate(context, echoProtoService);
+    GapicClass clazz =
+        HttpJsonServiceStubClassComposer.instance().generate(context, echoProtoService);
 
     JavaWriterVisitor visitor = new JavaWriterVisitor();
     clazz.classDefinition().accept(visitor);
@@ -51,49 +39,4 @@ public void generateServiceClasses() {
         Paths.get(Utils.getGoldenDir(this.getClass()), "HttpJsonComplianceStub.golden");
     Assert.assertCodeEquals(goldenFilePath, visitor.write());
   }
-
-  @Test
-  public void
-      getBindingFieldMethodName_shouldReturnGetFieldListIfTheFieldIsInLastPositionAndIsRepeated() {
-    Field field =
-        Field.builder()
-            .setIsRepeated(true)
-            .setName("doesNotMatter")
-            .setType(TypeNode.OBJECT)
-            .build();
-    HttpBinding httpBinding =
-        HttpBinding.builder().setField(field).setName("doesNotMatter").build();
-    String actual = composer.getBindingFieldMethodName(httpBinding, 4, 3, "Values");
-    Truth.assertThat(actual).isEqualTo("getValuesList");
-  }
-
-  @Test
-  public void
-      getBindingFieldMethodName_shouldReturnGetFieldValueIfTheFieldIsInLastPositionAndIsEnum() {
-    Field field =
-        Field.builder().setIsEnum(true).setName("doesNotMatter").setType(TypeNode.OBJECT).build();
-    HttpBinding httpBinding =
-        HttpBinding.builder().setField(field).setName("doesNotMatter").build();
-    String actual = composer.getBindingFieldMethodName(httpBinding, 4, 3, "Enums");
-    Truth.assertThat(actual).isEqualTo("getEnumsValue");
-  }
-
-  @Test
-  public void
-      getBindingFieldMethodName_shouldReturnGetFieldIfTheFieldIsInLastPositionAndNotRepeatedOrEnum() {
-    Field field = Field.builder().setName("doesNotMatter").setType(TypeNode.OBJECT).build();
-    HttpBinding httpBinding =
-        HttpBinding.builder().setField(field).setName("doesNotMatter").build();
-    String actual = composer.getBindingFieldMethodName(httpBinding, 4, 3, "Value");
-    Truth.assertThat(actual).isEqualTo("getValue");
-  }
-
-  @Test
-  public void getBindingFieldMethodName_shouldReturnGetFieldIfTheFieldIsNotInLastPosition() {
-    Field field = Field.builder().setName("doesNotMatter").setType(TypeNode.OBJECT).build();
-    HttpBinding httpBinding =
-        HttpBinding.builder().setField(field).setName("doesNotMatter").build();
-    String actual = composer.getBindingFieldMethodName(httpBinding, 4, 1, "Value");
-    Truth.assertThat(actual).isEqualTo("getValue");
-  }
 }

src/test/java/com/google/api/generator/gapic/composer/rest/goldens/ComplianceClientTest.golden

@@ -560,102 +560,4 @@ public class ComplianceClientTest {
       // Expected exception.
     }
   }
-
-  @Test
-  public void getEnumTest() throws Exception {
-    EnumResponse expectedResponse =
-        EnumResponse.newBuilder()
-            .setRequest(EnumRequest.newBuilder().build())
-            .setContinent(Continent.forNumber(0))
-            .build();
-    mockService.addResponse(expectedResponse);
-
-    EnumRequest request = EnumRequest.newBuilder().setUnknownEnum(true).build();
-
-    EnumResponse actualResponse = client.getEnum(request);
-    Assert.assertEquals(expectedResponse, actualResponse);
-
-    List<String> actualRequests = mockService.getRequestPaths();
-    Assert.assertEquals(1, actualRequests.size());
-
-    String apiClientHeaderKey =
-        mockService
-            .getRequestHeaders()
-            .get(ApiClientHeaderProvider.getDefaultApiClientHeaderKey())
-            .iterator()
-            .next();
-    Assert.assertTrue(
-        GaxHttpJsonProperties.getDefaultApiClientHeaderPattern()
-            .matcher(apiClientHeaderKey)
-            .matches());
-  }
-
-  @Test
-  public void getEnumExceptionTest() throws Exception {
-    ApiException exception =
-        ApiExceptionFactory.createException(
-            new Exception(), FakeStatusCode.of(StatusCode.Code.INVALID_ARGUMENT), false);
-    mockService.addException(exception);
-
-    try {
-      EnumRequest request = EnumRequest.newBuilder().setUnknownEnum(true).build();
-      client.getEnum(request);
-      Assert.fail("No exception raised");
-    } catch (InvalidArgumentException e) {
-      // Expected exception.
-    }
-  }
-
-  @Test
-  public void verifyEnumTest() throws Exception {
-    EnumResponse expectedResponse =
-        EnumResponse.newBuilder()
-            .setRequest(EnumRequest.newBuilder().build())
-            .setContinent(Continent.forNumber(0))
-            .build();
-    mockService.addResponse(expectedResponse);
-
-    EnumResponse request =
-        EnumResponse.newBuilder()
-            .setRequest(EnumRequest.newBuilder().build())
-            .setContinent(Continent.forNumber(0))
-            .build();
-
-    EnumResponse actualResponse = client.verifyEnum(request);
-    Assert.assertEquals(expectedResponse, actualResponse);
-
-    List<String> actualRequests = mockService.getRequestPaths();
-    Assert.assertEquals(1, actualRequests.size());
-
-    String apiClientHeaderKey =
-        mockService
-            .getRequestHeaders()
-            .get(ApiClientHeaderProvider.getDefaultApiClientHeaderKey())
-            .iterator()
-            .next();
-    Assert.assertTrue(
-        GaxHttpJsonProperties.getDefaultApiClientHeaderPattern()
-            .matcher(apiClientHeaderKey)
-            .matches());
-  }
-
-  @Test
-  public void verifyEnumExceptionTest() throws Exception {
-    ApiException exception =
-        ApiExceptionFactory.createException(
-            new Exception(), FakeStatusCode.of(StatusCode.Code.INVALID_ARGUMENT), false);
-    mockService.addException(exception);
-
-    try {
-      EnumResponse request =
-          EnumResponse.newBuilder()
-              .setRequest(EnumRequest.newBuilder().build())
-              .setContinent(Continent.forNumber(0))
-              .build();
-      client.verifyEnum(request);
-      Assert.fail("No exception raised");
-    } catch (InvalidArgumentException e) {
-      // Expected exception.
-    }
-  }
 }

src/test/java/com/google/api/generator/gapic/composer/rest/goldens/ComplianceSettings.golden

@@ -86,16 +86,6 @@ public class ComplianceSettings extends ClientSettings<ComplianceSettings> {
     return ((ComplianceStubSettings) getStubSettings()).repeatDataPathTrailingResourceSettings();
   }
 
-  /** Returns the object with the settings used for calls to getEnum. */
-  public UnaryCallSettings<EnumRequest, EnumResponse> getEnumSettings() {
-    return ((ComplianceStubSettings) getStubSettings()).getEnumSettings();
-  }
-
-  /** Returns the object with the settings used for calls to verifyEnum. */
-  public UnaryCallSettings<EnumResponse, EnumResponse> verifyEnumSettings() {
-    return ((ComplianceStubSettings) getStubSettings()).verifyEnumSettings();
-  }
-
   public static final ComplianceSettings create(ComplianceStubSettings stub) throws IOException {
     return new ComplianceSettings.Builder(stub.toBuilder()).build();
   }
@@ -225,16 +215,6 @@ public class ComplianceSettings extends ClientSettings<ComplianceSettings> {
       return getStubSettingsBuilder().repeatDataPathTrailingResourceSettings();
     }
 
-    /** Returns the builder for the settings used for calls to getEnum. */
-    public UnaryCallSettings.Builder<EnumRequest, EnumResponse> getEnumSettings() {
-      return getStubSettingsBuilder().getEnumSettings();
-    }
-
-    /** Returns the builder for the settings used for calls to verifyEnum. */
-    public UnaryCallSettings.Builder<EnumResponse, EnumResponse> verifyEnumSettings() {
-      return getStubSettingsBuilder().verifyEnumSettings();
-    }
-
     @Override
     public ComplianceSettings build() throws IOException {
       return new ComplianceSettings(this);