From 4ee028ef6b4c7df8da36394217032f74e3f18131 Mon Sep 17 00:00:00 2001
From: kolea2 <45548808+kolea2@users.noreply.github.com>
Date: Tue, 16 Jan 2024 12:02:03 -0500
Subject: [PATCH] feat: use existing transport options API to set transport
 (#1276)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* feat: use existing transport options API to set transport

* lint

* remove stale test

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
---
 README.md                                     |  6 +--
 .../cloud/datastore/DatastoreOptions.java     | 40 +++++++------------
 .../cloud/datastore/spi/v1/DatastoreRpc.java  |  8 ----
 .../testing/RemoteDatastoreHelper.java        |  5 +--
 .../cloud/datastore/DatastoreOptionsTest.java | 34 ++++++----------
 5 files changed, 32 insertions(+), 61 deletions(-)

diff --git a/README.md b/README.md
index ff9613efd..024f41511 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 0a590cc2e..c94e9ff7a 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.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 static class DefaultDatastoreRpcFactory implements DatastoreRpcFactory {
     @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 static class Builder extends ServiceOptions.Builder<Datastore, DatastoreO
 
     private String namespace;
     private String databaseId;
-    private Transport transport = GRPC;
 
     private Builder() {}
 
@@ -90,15 +91,10 @@ private Builder(DatastoreOptions options) {
       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 Builder setDatabaseId(String databaseId) {
       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 DatastoreRpcFactory getDefaultRpcFactory() {
 
     @Override
     public TransportOptions getDefaultTransportOptions() {
-      return getDefaultHttpTransportOptions();
+      return getDefaultGrpcTransportOptions();
     }
   }
 
@@ -165,6 +155,10 @@ public static HttpTransportOptions getDefaultHttpTransportOptions() {
     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 String getDatabaseId() {
     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 3b163f6fd..518bea983 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 @@ protected DatastoreSettings.Builder setInternalHeaderProvider(
       return super.setInternalHeaderProvider(internalHeaderProvider);
     }
   }
-
-  /** Transport used to sending requests. */
-  @InternalApi
-  enum Transport {
-    GRPC,
-    HTTP
-  }
 }
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 596ce96d8..cb1d96309 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.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 static RemoteDatastoreHelper create() {
 
   /** 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 6444b1a2d..6281befe6 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 void testDatastore() {
   @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 void testToBuilder() {
     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());
-    }
-  }
 }