fix(spring): extend hasRestOption fix to properties composer and add golden tests

src/main/java/com/google/api/generator/spring/composer/SpringPropertiesClassComposer.java

@@ -75,7 +75,18 @@ public GapicClass generate(GapicContext context, Service service) {
     String className = Utils.getServicePropertiesClassName(service);
     GapicServiceConfig gapicServiceConfig = context.serviceConfig();
     Map<String, TypeNode> dynamicTypes = createDynamicTypes(service, packageName);
-    boolean hasRestOption = context.transport().equals(Transport.GRPC_REST);
+    // TODO(emmwang): this condition is adapted from latest gapic-generator-java changes as part of
+    // https://github.com/googleapis/gapic-generator-java/issues/1117, but should be updated to use
+    // the gapic-implemented helpers once spring generator code is migrated.
+    boolean hasRestSupportedMethod =
+        service.methods().stream()
+            .anyMatch(
+                x ->
+                    x.httpBindings() != null
+                        && x.stream() != Method.Stream.BIDI
+                        && x.stream() != Method.Stream.CLIENT);
+    boolean hasRestOption =
+        context.transport().equals(Transport.GRPC_REST) && hasRestSupportedMethod;
 
     // TODO: this is the prefix user will use to set properties, may need to change depending on
     // branding.

src/main/java/com/google/api/generator/spring/composer/comment/SpringAutoconfigCommentComposer.java

@@ -42,7 +42,7 @@ public class SpringAutoconfigCommentComposer {
 
   public static final String TRANSPORT_CHANNEL_PROVIDER_GENERAL_DESCRIPTION =
       "Provides a default transport channel provider bean. The default is gRPC and will default to it unless the "
-          + "useRest option is provided to use HTTP transport instead";
+          + "useRest option is supported and provided to use HTTP transport instead";
   public static final String TRANSPORT_CHANNEL_PROVIDER_RETURN =
       "a default transport channel provider.";
   public static final String CLIENT_SETTINGS_BEAN_GENERAL_DESCRIPTION =

src/test/java/com/google/api/generator/gapic/composer/grpcrest/GrpcRestTestProtoLoader.java

@@ -31,6 +31,7 @@
 import com.google.protobuf.Descriptors.ServiceDescriptor;
 import com.google.protobuf.StructProto;
 import com.google.showcase.grpcrest.v1beta1.EchoGrpcrest;
+import com.google.showcase.v1beta1.WickedOuterClass;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.HashSet;
@@ -82,4 +83,35 @@ public GapicContext parseShowcaseEcho() {
         .setTransport(getTransport())
         .build();
   }
+
+  public GapicContext parseShowcaseWicked() {
+    FileDescriptor fileDescriptor = WickedOuterClass.getDescriptor();
+    ServiceDescriptor messagingService = fileDescriptor.getServices().get(0);
+    assertEquals("Wicked", messagingService.getName());
+
+    Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
+    messageTypes.putAll(Parser.parseMessages(OperationsProto.getDescriptor()));
+    messageTypes.putAll(Parser.parseMessages(StructProto.getDescriptor()));
+
+    Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
+    Set<ResourceName> outputResourceNames = new HashSet<>();
+    List<Service> services =
+        Parser.parseService(
+            fileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames);
+
+    String jsonFilename = "showcase_grpc_service_config.json";
+    Path jsonPath = Paths.get(getTestFilesDirectory(), jsonFilename);
+    Optional<GapicServiceConfig> configOpt = ServiceConfigParser.parse(jsonPath.toString());
+    assertTrue(configOpt.isPresent());
+    GapicServiceConfig config = configOpt.get();
+
+    return GapicContext.builder()
+        .setMessages(messageTypes)
+        .setResourceNames(resourceNames)
+        .setServices(services)
+        .setServiceConfig(config)
+        .setHelperResourceNames(outputResourceNames)
+        .setTransport(getTransport())
+        .build();
+  }
 }

src/test/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposerTest.java

@@ -15,6 +15,7 @@
 package com.google.api.generator.spring.composer;
 
 import com.google.api.generator.gapic.composer.common.TestProtoLoader;
+import com.google.api.generator.gapic.composer.grpcrest.GrpcRestTestProtoLoader;
 import com.google.api.generator.gapic.model.GapicClass;
 import com.google.api.generator.gapic.model.GapicContext;
 import com.google.api.generator.gapic.model.Service;
@@ -25,12 +26,16 @@
 
 public class SpringAutoConfigClassComposerTest {
   private GapicContext context;
+  private GapicContext wickedContext;
   private Service echoProtoService;
+  private Service wickedProtoService;
 
   @Before
   public void setUp() {
     this.context = TestProtoLoader.instance().parseShowcaseEcho();
     this.echoProtoService = this.context.services().get(0);
+    this.wickedContext = GrpcRestTestProtoLoader.instance().parseShowcaseWicked();
+    this.wickedProtoService = this.wickedContext.services().get(0);
   }
 
   @Test
@@ -43,12 +48,21 @@ public void generateAutoConfigClazzGrpcTest() {
 
   @Test
   public void generateAutoConfigClazzGrpcRestTest() {
-
     GapicContext contextGrpcRest =
         this.context.toBuilder().setTransport(Transport.GRPC_REST).build();
     GapicClass clazz =
         SpringAutoConfigClassComposer.instance().generate(contextGrpcRest, this.echoProtoService);
     String fileName = clazz.classDefinition().classIdentifier() + "GrpcRest.golden";
     Assert.assertGoldenClass(this.getClass(), clazz, fileName);
   }
+
+  @Test
+  public void generateAutoConfigClazzNoRestRpcsTest() {
+    GapicContext contextGrpcRest =
+        this.wickedContext.toBuilder().setTransport(Transport.GRPC_REST).build();
+    GapicClass clazz =
+        SpringAutoConfigClassComposer.instance().generate(contextGrpcRest, this.wickedProtoService);
+    String fileName = clazz.classDefinition().classIdentifier() + "NoRestRpcs.golden";
+    Assert.assertGoldenClass(this.getClass(), clazz, fileName);
+  }
 }

src/test/java/com/google/api/generator/spring/composer/SpringPropertiesClassComposerTest.java

@@ -15,6 +15,7 @@
 package com.google.api.generator.spring.composer;
 
 import com.google.api.generator.gapic.composer.common.TestProtoLoader;
+import com.google.api.generator.gapic.composer.grpcrest.GrpcRestTestProtoLoader;
 import com.google.api.generator.gapic.model.GapicClass;
 import com.google.api.generator.gapic.model.GapicContext;
 import com.google.api.generator.gapic.model.Service;
@@ -25,12 +26,16 @@
 
 public class SpringPropertiesClassComposerTest {
   private GapicContext context;
+  private GapicContext wickedContext;
   private Service echoProtoService;
+  private Service wickedProtoService;
 
   @Before
   public void setUp() {
     this.context = TestProtoLoader.instance().parseShowcaseEcho();
     this.echoProtoService = this.context.services().get(0);
+    this.wickedContext = GrpcRestTestProtoLoader.instance().parseShowcaseWicked();
+    this.wickedProtoService = this.wickedContext.services().get(0);
   }
 
   @Test
@@ -50,4 +55,14 @@ public void generatePropertiesClazzGrpcRestTest() {
     String fileName = clazz.classDefinition().classIdentifier() + "GrpcRest.golden";
     Assert.assertGoldenClass(this.getClass(), clazz, fileName);
   }
+
+  @Test
+  public void generatePropertiesClazzNoRestRpcsTest() {
+    GapicContext contextGrpcRest =
+        this.wickedContext.toBuilder().setTransport(Transport.GRPC_REST).build();
+    GapicClass clazz =
+        SpringPropertiesClassComposer.instance().generate(contextGrpcRest, this.wickedProtoService);
+    String fileName = clazz.classDefinition().classIdentifier() + "NoRestRpcs.golden";
+    Assert.assertGoldenClass(this.getClass(), clazz, fileName);
+  }
 }

src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden

@@ -86,7 +86,7 @@ public class EchoSpringAutoConfiguration {
 
   /**
    * Provides a default transport channel provider bean. The default is gRPC and will default to it
-   * unless the useRest option is provided to use HTTP transport instead
+   * unless the useRest option is supported and provided to use HTTP transport instead
    *
    * @return a default transport channel provider.
    */

src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden

@@ -66,7 +66,7 @@ public class EchoSpringAutoConfiguration {
 
   /**
    * Provides a default transport channel provider bean. The default is gRPC and will default to it
-   * unless the useRest option is provided to use HTTP transport instead
+   * unless the useRest option is supported and provided to use HTTP transport instead
    *
    * @return a default transport channel provider.
    */

src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden

@@ -67,7 +67,7 @@ public class EchoSpringAutoConfiguration {
 
   /**
    * Provides a default transport channel provider bean. The default is gRPC and will default to it
-   * unless the useRest option is provided to use HTTP transport instead
+   * unless the useRest option is supported and provided to use HTTP transport instead
    *
    * @return a default transport channel provider.
    */

src/test/java/com/google/api/generator/spring/composer/goldens/WickedSpringAutoConfigurationNoRestRpcs.golden

@@ -0,0 +1,164 @@
+package com.google.showcase.v1beta1.spring;
+
+import com.google.api.gax.core.CredentialsProvider;
+import com.google.api.gax.core.ExecutorProvider;
+import com.google.api.gax.retrying.RetrySettings;
+import com.google.api.gax.rpc.HeaderProvider;
+import com.google.api.gax.rpc.TransportChannelProvider;
+import com.google.cloud.spring.autoconfigure.core.GcpContextAutoConfiguration;
+import com.google.cloud.spring.core.Credentials;
+import com.google.cloud.spring.core.DefaultCredentialsProvider;
+import com.google.cloud.spring.core.Retry;
+import com.google.cloud.spring.core.util.RetryUtil;
+import com.google.showcase.v1beta1.WickedClient;
+import com.google.showcase.v1beta1.WickedSettings;
+import java.io.IOException;
+import java.util.Collections;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.boot.autoconfigure.AutoConfiguration;
+import org.springframework.boot.autoconfigure.AutoConfigureAfter;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.context.properties.EnableConfigurationProperties;
+import org.springframework.context.annotation.Bean;
+
+// AUTO-GENERATED DOCUMENTATION AND CLASS.
+/**
+ * Auto-configuration for {@link WickedClient}.
+ *
+ * <p>Provides auto-configuration for Spring Boot
+ *
+ * <p>The default instance has everything set to sensible defaults:
+ *
+ * <ul>
+ *   <li>The default transport provider is used.
+ *   <li>Credentials are acquired automatically through Application Default Credentials.
+ *   <li>Retries are configured for idempotent methods but not for non-idempotent methods.
+ * </ul>
+ */
+@AutoConfiguration
+@AutoConfigureAfter(GcpContextAutoConfiguration.class)
+@ConditionalOnClass(WickedClient.class)
+@ConditionalOnProperty(value = "com.google.showcase.v1beta1.wicked.enabled", matchIfMissing = true)
+@EnableConfigurationProperties(WickedSpringProperties.class)
+public class WickedSpringAutoConfiguration {
+  private final WickedSpringProperties clientProperties;
+  private final CredentialsProvider credentialsProvider;
+  private static final Log LOGGER = LogFactory.getLog(WickedSpringAutoConfiguration.class);
+
+  protected WickedSpringAutoConfiguration(
+      WickedSpringProperties clientProperties, CredentialsProvider credentialsProvider)
+      throws IOException {
+    this.clientProperties = clientProperties;
+    if (this.clientProperties.getCredentials().hasKey()) {
+      if (LOGGER.isTraceEnabled()) {
+        LOGGER.trace("Using credentials from Wicked-specific configuration");
+      }
+      this.credentialsProvider =
+          ((CredentialsProvider) new DefaultCredentialsProvider(this.clientProperties));
+    } else {
+      this.credentialsProvider = credentialsProvider;
+    }
+  }
+
+  /**
+   * Provides a default transport channel provider bean. The default is gRPC and will default to it
+   * unless the useRest option is supported and provided to use HTTP transport instead
+   *
+   * @return a default transport channel provider.
+   */
+  @Bean
+  @ConditionalOnMissingBean(name = "defaultWickedTransportChannelProvider")
+  public TransportChannelProvider defaultWickedTransportChannelProvider() {
+    return WickedSettings.defaultTransportChannelProvider();
+  }
+
+  /**
+   * Provides a WickedSettings bean configured to use a DefaultCredentialsProvider and the client
+   * library's default transport channel provider (defaultWickedTransportChannelProvider()). It also
+   * configures the quota project ID and executor thread count, if provided through properties.
+   *
+   * <p>Retry settings are also configured from service-level and method-level properties specified
+   * in WickedSpringProperties. Method-level properties will take precedence over service-level
+   * properties if available, and client library defaults will be used if neither are specified.
+   *
+   * @param defaultTransportChannelProvider TransportChannelProvider to use in the settings.
+   * @return a {@link WickedSettings} bean configured with {@link TransportChannelProvider} bean.
+   */
+  @Bean
+  @ConditionalOnMissingBean
+  public WickedSettings wickedSettings(
+      @Qualifier("defaultWickedTransportChannelProvider")
+          TransportChannelProvider defaultTransportChannelProvider)
+      throws IOException {
+    WickedSettings.Builder clientSettingsBuilder = WickedSettings.newBuilder();
+    clientSettingsBuilder
+        .setCredentialsProvider(this.credentialsProvider)
+        .setTransportChannelProvider(defaultTransportChannelProvider)
+        .setHeaderProvider(this.userAgentHeaderProvider());
+    if (this.clientProperties.getQuotaProjectId() != null) {
+      clientSettingsBuilder.setQuotaProjectId(this.clientProperties.getQuotaProjectId());
+      if (LOGGER.isTraceEnabled()) {
+        LOGGER.trace(
+            "Quota project id set to "
+                + this.clientProperties.getQuotaProjectId()
+                + ", this overrides project id from credentials.");
+      }
+    }
+    if (this.clientProperties.getExecutorThreadCount() != null) {
+      ExecutorProvider executorProvider =
+          WickedSettings.defaultExecutorProviderBuilder()
+              .setExecutorThreadCount(this.clientProperties.getExecutorThreadCount())
+              .build();
+      clientSettingsBuilder.setBackgroundExecutorProvider(executorProvider);
+      if (LOGGER.isTraceEnabled()) {
+        LOGGER.trace(
+            "Background executor thread count is "
+                + this.clientProperties.getExecutorThreadCount());
+      }
+    }
+    Retry serviceRetry = clientProperties.getRetry();
+    if (serviceRetry != null) {
+      RetrySettings craftEvilPlanRetrySettings =
+          RetryUtil.updateRetrySettings(
+              clientSettingsBuilder.craftEvilPlanSettings().getRetrySettings(), serviceRetry);
+      clientSettingsBuilder.craftEvilPlanSettings().setRetrySettings(craftEvilPlanRetrySettings);
+
+      if (LOGGER.isTraceEnabled()) {
+        LOGGER.trace("Configured service-level retry settings from properties.");
+      }
+    }
+    Retry craftEvilPlanRetry = clientProperties.getCraftEvilPlanRetry();
+    if (craftEvilPlanRetry != null) {
+      RetrySettings craftEvilPlanRetrySettings =
+          RetryUtil.updateRetrySettings(
+              clientSettingsBuilder.craftEvilPlanSettings().getRetrySettings(), craftEvilPlanRetry);
+      clientSettingsBuilder.craftEvilPlanSettings().setRetrySettings(craftEvilPlanRetrySettings);
+      if (LOGGER.isTraceEnabled()) {
+        LOGGER.trace("Configured method-level retry settings for craftEvilPlan from properties.");
+      }
+    }
+    return clientSettingsBuilder.build();
+  }
+
+  /**
+   * Provides a WickedClient bean configured with WickedSettings.
+   *
+   * @param wickedSettings settings to configure an instance of client bean.
+   * @return a {@link WickedClient} bean configured with {@link WickedSettings}
+   */
+  @Bean
+  @ConditionalOnMissingBean
+  public WickedClient wickedClient(WickedSettings wickedSettings) throws IOException {
+    return WickedClient.create(wickedSettings);
+  }
+
+  private HeaderProvider userAgentHeaderProvider() {
+    String springLibrary = "spring-autogen-wicked";
+    String version = this.getClass().getPackage().getImplementationVersion();
+    return () -> Collections.singletonMap("user-agent", springLibrary + "/" + version);
+  }
+}