Repository: googleapis/java-datastore
Pull request #1276 (2024-01-04 16:37:53)

feat: use existing transport options API to set transport
Building on top of https://github.com/googleapis/java-datastore/pull/1261, `setTransportOptions` has always been available as an API to use. Instead of writing custom logic like `setTransport`, we will use the existing API instead and allow it to be set to either gRPC or HTTP.


Patch:
diff --git a/README.md b/README.md
index ff9613ef..024f4151 100644
--- a/README.md
+++ b/README.md
@@ -57,13 +57,13 @@ implementation 'com.google.cloud:google-cloud-datastore'
 If you are using Gradle without BOM, add this to your dependencies:
 
 ```Groovy
-implementation 'com.google.cloud:google-cloud-datastore:2.17.6'
+implementation 'com.google.cloud:google-cloud-datastore:2.18.0'
 ```
 
 If you are using SBT, add this to your dependencies:
 
 ```Scala
-libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.17.6"
+libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.18.0"
 ```
 <!-- {x-version-update-end} -->
 
@@ -380,7 +380,7 @@ Java is a registered trademark of Oracle and/or its affiliates.
 [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-datastore/java11.html
 [stability-image]: https://img.shields.io/badge/stability-stable-green
 [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-datastore.svg
-[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-datastore/2.17.6
+[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-datastore/2.18.0
 [authentication]: https://github.com/googleapis/google-cloud-java#authentication
 [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes
 [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles
diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java
index 0a590cc2..c94e9ff7 100644
--- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java
+++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java
@@ -17,7 +17,6 @@
 package com.google.cloud.datastore;
 
 import static com.google.cloud.datastore.Validator.validateNamespace;
-import static com.google.cloud.datastore.spi.v1.DatastoreRpc.Transport.GRPC;
 
 import com.google.cloud.ServiceDefaults;
 import com.google.cloud.ServiceOptions;
@@ -25,12 +24,11 @@ import com.google.cloud.ServiceRpc;
 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.spi.v1.DatastoreRpc.Transport;
 import com.google.cloud.datastore.spi.v1.GrpcDatastoreRpc;
 import com.google.cloud.datastore.spi.v1.HttpDatastoreRpc;
 import com.google.cloud.datastore.v1.DatastoreSettings;
+import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.http.HttpTransportOptions;
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableSet;
 import java.io.IOException;
@@ -50,7 +48,6 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
 
   private final String namespace;
   private final String databaseId;
-  private final Transport transport;
 
   public static class DefaultDatastoreFactory implements DatastoreFactory {
 
@@ -69,9 +66,14 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
     @Override
     public ServiceRpc create(DatastoreOptions options) {
       try {
-        return options.transport == GRPC
-            ? new GrpcDatastoreRpc(options)
-            : new HttpDatastoreRpc(options);
+        if (options.getTransportOptions() instanceof GrpcTransportOptions) {
+          return new GrpcDatastoreRpc(options);
+        } else if (options.getTransportOptions() instanceof HttpTransportOptions) {
+          return new HttpDatastoreRpc(options);
+        } else {
+          throw new IllegalArgumentException(
+              "unknown transport type: " + options.getTransportOptions());
+        }
       } catch (IOException e) {
         throw new RuntimeException(e);
       }
@@ -82,7 +84,6 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
 
     private String namespace;
     private String databaseId;
-    private Transport transport = GRPC;
 
     private Builder() {}
 
@@ -90,15 +91,10 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
       super(options);
       namespace = options.namespace;
       databaseId = options.databaseId;
-      transport = options.transport;
     }
 
     @Override
     public Builder setTransportOptions(TransportOptions transportOptions) {
-      if (!(transportOptions instanceof HttpTransportOptions)) {
-        throw new IllegalArgumentException(
-            "Only http transport is allowed for " + API_SHORT_NAME + ".");
-      }
       return super.setTransportOptions(transportOptions);
     }
 
@@ -117,18 +113,12 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
       this.databaseId = databaseId;
       return this;
     }
-
-    public Builder setTransport(Transport transport) {
-      this.transport = transport;
-      return this;
-    }
   }
 
   private DatastoreOptions(Builder builder) {
     super(DatastoreFactory.class, DatastoreRpcFactory.class, builder, new DatastoreDefaults());
     namespace = MoreObjects.firstNonNull(builder.namespace, defaultNamespace());
     databaseId = MoreObjects.firstNonNull(builder.databaseId, DEFAULT_DATABASE_ID);
-    transport = builder.transport;
   }
 
   @Override
@@ -157,7 +147,7 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
 
     @Override
     public TransportOptions getDefaultTransportOptions() {
-      return getDefaultHttpTransportOptions();
+      return getDefaultGrpcTransportOptions();
     }
   }
 
@@ -165,6 +155,10 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
     return HttpTransportOptions.newBuilder().build();
   }
 
+  public static GrpcTransportOptions getDefaultGrpcTransportOptions() {
+    return GrpcTransportOptions.newBuilder().build();
+  }
+
   /** Returns the default namespace to be used by the datastore service. */
   public String getNamespace() {
     return namespace;
@@ -174,12 +168,6 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
     return this.databaseId;
   }
 
-  /** Returns the current transport */
-  @VisibleForTesting
-  Transport getTransport() {
-    return this.transport;
-  }
-
   /** Returns a default {@code DatastoreOptions} instance. */
   public static DatastoreOptions getDefaultInstance() {
     return newBuilder().build();
diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/DatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/DatastoreRpc.java
index 3b163f6f..518bea98 100644
--- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/DatastoreRpc.java
+++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/DatastoreRpc.java
@@ -16,7 +16,6 @@
 
 package com.google.cloud.datastore.spi.v1;
 
-import com.google.api.core.InternalApi;
 import com.google.api.core.InternalExtensionOnly;
 import com.google.api.gax.rpc.HeaderProvider;
 import com.google.cloud.ServiceRpc;
@@ -117,11 +116,4 @@ public interface DatastoreRpc extends ServiceRpc, AutoCloseable {
       return super.setInternalHeaderProvider(internalHeaderProvider);
     }
   }
-
-  /** Transport used to sending requests. */
-  @InternalApi
-  enum Transport {
-    GRPC,
-    HTTP
-  }
 }


Test patch:
diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java
index 596ce96d..cb1d9630 100644
--- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java
+++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java
@@ -24,6 +24,7 @@ import com.google.cloud.datastore.Key;
 import com.google.cloud.datastore.Query;
 import com.google.cloud.datastore.QueryResults;
 import com.google.cloud.datastore.StructuredQuery;
+import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.http.HttpTransportOptions;
 import java.util.UUID;
 import org.threeten.bp.Duration;
@@ -79,9 +80,7 @@ public class RemoteDatastoreHelper {
 
   /** Creates a {@code RemoteStorageHelper} object. */
   public static RemoteDatastoreHelper create(String databaseId) {
-    HttpTransportOptions transportOptions = DatastoreOptions.getDefaultHttpTransportOptions();
-    transportOptions =
-        transportOptions.toBuilder().setConnectTimeout(60000).setReadTimeout(60000).build();
+    GrpcTransportOptions transportOptions = DatastoreOptions.getDefaultGrpcTransportOptions();
     DatastoreOptions datastoreOption =
         DatastoreOptions.newBuilder()
             .setDatabaseId(databaseId)
diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java
index 6444b1a2..6281befe 100644
--- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java
+++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java
@@ -16,20 +16,17 @@
 
 package com.google.cloud.datastore;
 
-import static com.google.cloud.datastore.spi.v1.DatastoreRpc.Transport.GRPC;
-import static com.google.cloud.datastore.spi.v1.DatastoreRpc.Transport.HTTP;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
-import com.google.cloud.TransportOptions;
 import com.google.cloud.datastore.spi.DatastoreRpcFactory;
 import com.google.cloud.datastore.spi.v1.DatastoreRpc;
+import com.google.cloud.grpc.GrpcTransportOptions;
+import com.google.cloud.http.HttpTransportOptions;
 import org.easymock.EasyMock;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -87,17 +84,23 @@ public class DatastoreOptionsTest {
   @Test
   public void testTransport() {
     // default grpc transport
-    assertThat(options.build().getTransport()).isEqualTo(GRPC);
+    assertThat(options.build().getTransportOptions()).isInstanceOf(GrpcTransportOptions.class);
 
     // custom http transport
     DatastoreOptions httpDatastoreOptions =
-        DatastoreOptions.newBuilder().setTransport(HTTP).setProjectId(PROJECT_ID).build();
-    assertThat(httpDatastoreOptions.getTransport()).isEqualTo(HTTP);
+        DatastoreOptions.newBuilder()
+            .setTransportOptions(HttpTransportOptions.newBuilder().build())
+            .setProjectId(PROJECT_ID)
+            .build();
+    assertThat(httpDatastoreOptions.getTransportOptions()).isInstanceOf(HttpTransportOptions.class);
 
     // custom grpc transport
     DatastoreOptions grpcDatastoreOptions =
-        DatastoreOptions.newBuilder().setTransport(GRPC).setProjectId(PROJECT_ID).build();
-    assertThat(grpcDatastoreOptions.getTransport()).isEqualTo(GRPC);
+        DatastoreOptions.newBuilder()
+            .setTransportOptions(GrpcTransportOptions.newBuilder().build())
+            .setProjectId(PROJECT_ID)
+            .build();
+    assertThat(grpcDatastoreOptions.getTransportOptions()).isInstanceOf(GrpcTransportOptions.class);
   }
 
   @Test
@@ -116,15 +119,4 @@ public class DatastoreOptionsTest {
     assertNotEquals(original, newOptions);
     assertNotEquals(original.hashCode(), newOptions.hashCode());
   }
-
-  @Test
-  public void testInvalidTransport() {
-    try {
-      DatastoreOptions.newBuilder()
-          .setTransportOptions(EasyMock.createMock(TransportOptions.class));
-      Assert.fail();
-    } catch (IllegalArgumentException ex) {
-      assertNotNull(ex.getMessage());
-    }
-  }
 }