feat(Datastore): Swap the default transport from HttpJson to gRPC

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java

@@ -19,7 +19,6 @@
 import static com.google.cloud.datastore.Validator.validateNamespace;
 
 import com.google.api.core.BetaApi;
-import com.google.api.core.ObsoleteApi;
 import com.google.api.gax.grpc.ChannelPoolSettings;
 import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
 import com.google.api.gax.rpc.TransportChannelProvider;
@@ -35,18 +34,21 @@
 import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.http.HttpTransportOptions;
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.util.Objects;
 import java.util.Set;
+import java.util.logging.Logger;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions> {
 
   private static final long serialVersionUID = -1018382430058137336L;
   private static final String API_SHORT_NAME = "Datastore";
+  private static final Logger logger = Logger.getLogger(DatastoreOptions.class.getName());
   private static final String DATASTORE_SCOPE = "https://www.googleapis.com/auth/datastore";
   private static final Set<String> SCOPES = ImmutableSet.of(DATASTORE_SCOPE);
   private static final String DEFAULT_DATABASE_ID = "";
@@ -72,9 +74,7 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
   /**
    * @deprecated This constant is obsolete and will be removed in a future version.
    */
-  @ObsoleteApi("This constant is obsolete and will be removed in a future version.")
-  @Deprecated
-  public static final int MAX_CHANNEL_COUNT = 10;
+  @Deprecated public static final int MAX_CHANNEL_COUNT = 10;
 
   private transient TransportChannelProvider channelProvider = null;
 
@@ -129,7 +129,10 @@ public static class Builder extends ServiceOptions.Builder<Datastore, DatastoreO
     private String databaseId;
     private TransportChannelProvider channelProvider = null;
     private String host;
-    private TransportOptions transportOptions;
+
+    @Nonnull
+    private TransportOptions transportOptions =
+        new DatastoreDefaults().getDefaultTransportOptions();
 
     @Nullable private DatastoreOpenTelemetryOptions openTelemetryOptions = null;
 
@@ -141,25 +144,51 @@ private Builder(DatastoreOptions options) {
       this.databaseId = options.databaseId;
       this.openTelemetryOptions = options.openTelemetryOptions;
       this.channelProvider = validateChannelProvider(options.channelProvider);
+      this.host = options.getHost();
+      this.transportOptions = options.getTransportOptions();
+    }
+
+    private TransportChannelProvider validateChannelProvider(
+        TransportChannelProvider channelProvider) {
+      Preconditions.checkNotNull(channelProvider, "TransportChannelProvider cannot be null");
+      if (!(channelProvider instanceof InstantiatingGrpcChannelProvider)) {
+        throw new IllegalArgumentException(
+            "Only GRPC channels are allowed for " + API_SHORT_NAME + ".");
+      }
+      return channelProvider;
     }
 
+    /**
+     * Sets the transport options.
+     *
+     * @param transportOptions the transport options to set, must be {@link HttpTransportOptions} or
+     *     {@link GrpcTransportOptions}
+     * @return the builder
+     * @throws IllegalArgumentException if the transport options are not supported
+     */
     @Override
-    public Builder setTransportOptions(TransportOptions transportOptions) {
-      if (!(transportOptions instanceof HttpTransportOptions)) {
+    public Builder setTransportOptions(@Nonnull TransportOptions transportOptions) {
+      Preconditions.checkNotNull(transportOptions, "TransportOptions cannot be null");
+      if (!(transportOptions instanceof HttpTransportOptions)
+          && !(transportOptions instanceof GrpcTransportOptions)) {
         throw new IllegalArgumentException(
-            "Only http transport is allowed for " + API_SHORT_NAME + ".");
+            "Only http and grpc transport are allowed for " + API_SHORT_NAME + ".");
       }
       this.transportOptions = transportOptions;
       return super.setTransportOptions(transportOptions);
     }
 
     /**
-     * Sets the transport to gRPC. Note this functionality is experimental and subject to change.
+     * This method deprecated. Prefer to use {@link #setTransportOptions(TransportOptions)} instead.
+     * When using the transport-neutral variant, you may need to cast to TransportOptions when using
+     * a GrpcTransportOptions class, otherwise it will default to the deprecated method.
+     *
+     * <p>Sets the transport to gRPC. Note this functionality is experimental and subject to change.
      */
+    @Deprecated
     @BetaApi
     public Builder setTransportOptions(GrpcTransportOptions transportOptions) {
-      this.transportOptions = transportOptions;
-      return super.setTransportOptions(transportOptions);
+      return setTransportOptions((TransportOptions) transportOptions);
     }
 
     @Override
@@ -185,10 +214,28 @@ public Builder setChannelProvider(TransportChannelProvider channelProvider) {
       return this;
     }
 
+    /**
+     * Builds the {@link DatastoreOptions} instance.
+     *
+     * <p>If the host is not explicitly set, it defaults to the transport-specific default host:
+     *
+     * <ul>
+     *   <li>gRPC: {@code datastore.googleapis.com:443}
+     *   <li>HTTP: {@code https://datastore.googleapis.com}
+     * </ul>
+     *
+     * @return the {@link DatastoreOptions} instance
+     */
     @Override
     public DatastoreOptions build() {
-      if (this.host == null && this.transportOptions instanceof GrpcTransportOptions) {
-        this.setHost(DatastoreSettings.getDefaultEndpoint());
+      if (this.host == null) {
+        // Use whatever host value the user passes in, otherwise use the transport specific default
+        // host values
+        if (this.transportOptions instanceof GrpcTransportOptions) {
+          this.setHost(DatastoreSettings.getDefaultEndpoint());
+        } else if (this.transportOptions instanceof HttpTransportOptions) {
+          this.setHost(com.google.datastore.v1.client.DatastoreFactory.DEFAULT_HOST);
+        }
       }
       return new DatastoreOptions(this);
     }
@@ -218,15 +265,6 @@ public Builder setOpenTelemetryOptions(
     }
   }
 
-  private static TransportChannelProvider validateChannelProvider(
-      TransportChannelProvider channelProvider) {
-    if (channelProvider != null && !(channelProvider instanceof InstantiatingGrpcChannelProvider)) {
-      throw new IllegalArgumentException(
-          "Only GRPC channels are allowed for " + API_SHORT_NAME + ".");
-    }
-    return channelProvider;
-  }
-
   private DatastoreOptions(Builder builder) {
     super(DatastoreFactory.class, DatastoreRpcFactory.class, builder, new DatastoreDefaults());
 
@@ -239,9 +277,9 @@ private DatastoreOptions(Builder builder) {
     namespace = MoreObjects.firstNonNull(builder.namespace, defaultNamespace());
     databaseId = MoreObjects.firstNonNull(builder.databaseId, DEFAULT_DATABASE_ID);
 
+    // ChannelProvider is used by GAX but HttpJson does not use it so we safely ignore it.
     if (getTransportOptions() instanceof HttpTransportOptions && builder.channelProvider != null) {
-      throw new IllegalArgumentException(
-          "Only gRPC transport allows setting of channel provider or credentials provider");
+      logger.warning("Channel provider is ignored for HttpJson transport.");
     } else if (getTransportOptions() instanceof GrpcTransportOptions) {
       if (builder.channelProvider == null) {
         // Set the default gRPC connection pool to be configured with a minimum of 1 channel.
@@ -310,8 +348,8 @@ public TransportOptions getDefaultTransportOptions() {
       return TRANSPORT_OPTIONS;
     }
 
-    public static HttpTransportOptions.Builder getDefaultTransportOptionsBuilder() {
-      return HttpTransportOptions.newBuilder();
+    public static GrpcTransportOptions.Builder getDefaultTransportOptionsBuilder() {
+      return GrpcTransportOptions.newBuilder();
     }
   }
 

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java

@@ -22,6 +22,7 @@
 import com.google.api.core.InternalApi;
 import com.google.cloud.NoCredentials;
 import com.google.cloud.ServiceOptions;
+import com.google.cloud.TransportOptions;
 import com.google.cloud.datastore.DatastoreOptions;
 import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.testing.BaseEmulatorHelper;
@@ -242,11 +243,14 @@ public DatastoreOptions getOptions(String namespace) {
   }
 
   /**
-   * Returns a {@link DatastoreOptions} instance that sets the host to use the Datastore emulator on
-   * localhost. The transportOptions is set to {@code grpcTransportOptions}.
+   * Prefer to set the TransportOptions via the Options Builder.
+   *
+   * <p>Returns a {@link DatastoreOptions} instance that sets the host to use the Datastore emulator
+   * on localhost. The transportOptions is set to {@code grpcTransportOptions}.
    */
+  @Deprecated
   public DatastoreOptions getGrpcTransportOptions(GrpcTransportOptions grpcTransportOptions) {
-    return optionsBuilder.setTransportOptions(grpcTransportOptions).build();
+    return optionsBuilder.setTransportOptions((TransportOptions) grpcTransportOptions).build();
   }
 
   public DatastoreOptions.Builder setNamespace(String namespace) {

java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java

@@ -21,6 +21,7 @@
 import com.google.api.gax.rpc.StatusCode;
 import com.google.cloud.NoCredentials;
 import com.google.cloud.ServiceOptions;
+import com.google.cloud.TransportOptions;
 import com.google.cloud.datastore.spi.DatastoreRpcFactory;
 import com.google.cloud.datastore.spi.v1.DatastoreRpc;
 import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder;
@@ -115,7 +116,7 @@ public void setUp() {
                     .build());
 
     if (TelemetryConstants.Transport.GRPC.equals(transport)) {
-      builder.setTransportOptions(GrpcTransportOptions.newBuilder().build());
+      builder.setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build());
     } else {
       builder.setTransportOptions(HttpTransportOptions.newBuilder().build());
     }

java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java

@@ -27,14 +27,14 @@
 import com.google.api.gax.grpc.ChannelPoolSettings;
 import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
 import com.google.cloud.NoCredentials;
+import com.google.cloud.TransportOptions;
 import com.google.cloud.datastore.spi.DatastoreRpcFactory;
 import com.google.cloud.datastore.spi.v1.DatastoreRpc;
 import com.google.cloud.datastore.v1.DatastoreSettings;
 import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.http.HttpTransportOptions;
 import com.google.datastore.v1.client.DatastoreFactory;
 import org.easymock.EasyMock;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -217,7 +217,7 @@ public void testGrpcDefaultChannelConfigurations() {
             .setServiceRpcFactory(datastoreRpcFactory)
             .setProjectId(PROJECT_ID)
             .setDatabaseId(DATABASE_ID)
-            .setTransportOptions(GrpcTransportOptions.newBuilder().build())
+            .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build())
             .setCredentials(NoCredentials.getInstance())
             .setHost("http://localhost:" + PORT)
             .build();
@@ -250,7 +250,7 @@ public void testCustomChannelAndCredentials() {
             .setServiceRpcFactory(datastoreRpcFactory)
             .setProjectId(PROJECT_ID)
             .setDatabaseId(DATABASE_ID)
-            .setTransportOptions(GrpcTransportOptions.newBuilder().build())
+            .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build())
             .setChannelProvider(channelProvider)
             .setCredentials(NoCredentials.getInstance())
             .setHost("http://localhost:" + PORT)
@@ -259,35 +259,23 @@ public void testCustomChannelAndCredentials() {
   }
 
   @Test
-  public void testInvalidConfigForHttp() {
-    DatastoreOptions.Builder options =
+  public void testTransport() {
+    // default grpc transport
+    assertThat(options.build().getTransportOptions()).isInstanceOf(GrpcTransportOptions.class);
+
+    // custom http transport
+    DatastoreOptions httpTransportOptions =
         DatastoreOptions.newBuilder()
-            .setServiceRpcFactory(datastoreRpcFactory)
-            .setProjectId(PROJECT_ID)
-            .setDatabaseId(DATABASE_ID)
             .setTransportOptions(HttpTransportOptions.newBuilder().build())
-            .setChannelProvider(
-                DatastoreSettings.defaultGrpcTransportProviderBuilder()
-                    .setChannelPoolSettings(
-                        ChannelPoolSettings.builder()
-                            .setInitialChannelCount(10)
-                            .setMaxChannelCount(20)
-                            .build())
-                    .build())
+            .setProjectId(PROJECT_ID)
             .setCredentials(NoCredentials.getInstance())
-            .setHost("http://localhost:" + PORT);
-    Assert.assertThrows(IllegalArgumentException.class, options::build);
-  }
-
-  @Test
-  public void testTransport() {
-    // default http transport
-    assertThat(options.build().getTransportOptions()).isInstanceOf(HttpTransportOptions.class);
+            .build();
+    assertThat(httpTransportOptions.getTransportOptions()).isInstanceOf(HttpTransportOptions.class);
 
     // custom grpc transport
     DatastoreOptions grpcTransportOptions =
         DatastoreOptions.newBuilder()
-            .setTransportOptions(GrpcTransportOptions.newBuilder().build())
+            .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build())
             .setProjectId(PROJECT_ID)
             .setCredentials(NoCredentials.getInstance())
             .build();
@@ -300,7 +288,7 @@ public void testTransport() {
   public void testHostWithGrpcAndHttp() {
     DatastoreOptions grpcTransportOptions =
         DatastoreOptions.newBuilder()
-            .setTransportOptions(GrpcTransportOptions.newBuilder().build())
+            .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build())
             .setProjectId(PROJECT_ID)
             .setCredentials(NoCredentials.getInstance())
             .build();
@@ -310,22 +298,31 @@ public void testHostWithGrpcAndHttp() {
     String customHost = "http://localhost:" + PORT;
     DatastoreOptions grpcTransportOptionsCustomHost =
         DatastoreOptions.newBuilder()
-            .setTransportOptions(GrpcTransportOptions.newBuilder().build())
+            .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build())
             .setHost(customHost)
             .setProjectId(PROJECT_ID)
             .setCredentials(NoCredentials.getInstance())
             .build();
     assertThat(grpcTransportOptionsCustomHost.getHost()).isEqualTo(customHost);
 
+    DatastoreOptions defaultTransportOptions =
+        DatastoreOptions.newBuilder()
+            .setProjectId(PROJECT_ID)
+            .setCredentials(NoCredentials.getInstance())
+            .build();
+    assertThat(defaultTransportOptions.getHost()).isEqualTo(DatastoreSettings.getDefaultEndpoint());
+
     DatastoreOptions httpTransportOptions =
         DatastoreOptions.newBuilder()
+            .setTransportOptions(HttpTransportOptions.newBuilder().build())
             .setProjectId(PROJECT_ID)
             .setCredentials(NoCredentials.getInstance())
             .build();
     assertThat(httpTransportOptions.getHost()).isEqualTo(DatastoreFactory.DEFAULT_HOST);
 
     DatastoreOptions httpTransportOptionsCustomHost =
         DatastoreOptions.newBuilder()
+            .setTransportOptions(HttpTransportOptions.newBuilder().build())
             .setHost(customHost)
             .setProjectId(PROJECT_ID)
             .setCredentials(NoCredentials.getInstance())

java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestGrpc.java

@@ -17,7 +17,6 @@
 package com.google.cloud.datastore;
 
 import com.google.cloud.datastore.testing.LocalDatastoreHelper;
-import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.common.truth.Truth;
 import java.io.IOException;
 import java.time.Duration;
@@ -32,8 +31,7 @@ public class DatastoreTestGrpc extends AbstractDatastoreTest {
 
   private static final LocalDatastoreHelper helper = LocalDatastoreHelper.create(1.0, 9090);
 
-  private static DatastoreOptions options =
-      helper.getGrpcTransportOptions(GrpcTransportOptions.newBuilder().build());
+  private static DatastoreOptions options = helper.getOptions();
   private static Datastore datastore = options.getService();
 
   public DatastoreTestGrpc(DatastoreOptions options, Datastore datastore) {
@@ -48,7 +46,7 @@ public static Iterable<Object[]> data() {
   @BeforeClass
   public static void beforeClass() throws IOException, InterruptedException {
     helper.start();
-    options = helper.getGrpcTransportOptions(GrpcTransportOptions.newBuilder().build());
+    options = helper.getOptions();
     datastore = options.getService();
   }
 

java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestHttp.java

@@ -17,7 +17,7 @@
 package com.google.cloud.datastore;
 
 import com.google.cloud.datastore.testing.LocalDatastoreHelper;
-import com.google.cloud.grpc.GrpcTransportOptions;
+import com.google.cloud.http.HttpTransportOptions;
 import java.io.IOException;
 import java.time.Duration;
 import java.util.Arrays;
@@ -46,7 +46,10 @@ public static Iterable<Object[]> data() {
   @BeforeClass
   public static void beforeClass() throws IOException, InterruptedException {
     helper.start();
-    options = helper.getGrpcTransportOptions(GrpcTransportOptions.newBuilder().build());
+    options =
+        helper.getOptions().toBuilder()
+            .setTransportOptions(HttpTransportOptions.newBuilder().build())
+            .build();
     datastore = options.getService();
   }