Repository: googleapis/java-datastore
Pull request #1299 (2024-01-24 16:51:47)

feat: allow setting channel and credential providers in DatastoreOptions
Similar to Firestore, https://github.com/googleapis/java-firestore/blob/main/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java#L162-L168, this allows users to configure more custom options available in gax and gRPC like credentials providers and channel pooling.


Patch:
diff --git a/README.md b/README.md
index 024f4151..e4f70855 100644
--- a/README.md
+++ b/README.md
@@ -50,20 +50,20 @@ If you are using Maven without the BOM, add this to your dependencies:
 If you are using Gradle 5.x or later, add this to your dependencies:
 
 ```Groovy
-implementation platform('com.google.cloud:libraries-bom:26.29.0')
+implementation platform('com.google.cloud:libraries-bom:26.31.0')
 
 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.18.0'
+implementation 'com.google.cloud:google-cloud-datastore:2.18.3'
 ```
 
 If you are using SBT, add this to your dependencies:
 
 ```Scala
-libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.18.0"
+libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.18.3"
 ```
 <!-- {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.18.0
+[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-datastore/2.18.3
 [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/datastore-v1-proto-client/pom.xml b/datastore-v1-proto-client/pom.xml
index ebde7c64..142798e9 100644
--- a/datastore-v1-proto-client/pom.xml
+++ b/datastore-v1-proto-client/pom.xml
@@ -98,7 +98,7 @@
     <dependency>
       <groupId>com.google.truth</groupId>
       <artifactId>truth</artifactId>
-      <version>1.1.5</version>
+      <version>1.4.0</version>
       <scope>test</scope>
       <exclusions>
         <exclusion>
diff --git a/google-cloud-datastore-bom/pom.xml b/google-cloud-datastore-bom/pom.xml
index 2105a8ca..8620ce3a 100644
--- a/google-cloud-datastore-bom/pom.xml
+++ b/google-cloud-datastore-bom/pom.xml
@@ -8,7 +8,7 @@
   <parent>
     <groupId>com.google.cloud</groupId>
     <artifactId>google-cloud-shared-config</artifactId>
-    <version>1.6.1</version>
+    <version>1.7.1</version>
   </parent>
 
   <name>Google Cloud datastore BOM</name>
diff --git a/google-cloud-datastore/pom.xml b/google-cloud-datastore/pom.xml
index 114e4d40..3f13224d 100644
--- a/google-cloud-datastore/pom.xml
+++ b/google-cloud-datastore/pom.xml
@@ -118,6 +118,13 @@
       <type>test-jar</type>
       <scope>test</scope>
     </dependency>
+
+    <dependency>
+      <groupId>com.google.guava</groupId>
+      <artifactId>guava-testlib</artifactId>
+      <version>33.0.0-jre</version>
+      <scope>test</scope>
+    </dependency>
     <!-- Need testing utility classes for generated gRPC clients tests -->
     <dependency>
       <groupId>com.google.api</groupId>
@@ -150,7 +157,7 @@
     <dependency>
       <groupId>com.google.truth</groupId>
       <artifactId>truth</artifactId>
-      <version>1.1.5</version>
+      <version>1.4.0</version>
       <scope>test</scope>
     </dependency>
   </dependencies>
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 c94e9ff7..c852fdb7 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
@@ -18,6 +18,9 @@ package com.google.cloud.datastore;
 
 import static com.google.cloud.datastore.Validator.validateNamespace;
 
+import com.google.api.gax.core.CredentialsProvider;
+import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
+import com.google.api.gax.rpc.TransportChannelProvider;
 import com.google.cloud.ServiceDefaults;
 import com.google.cloud.ServiceOptions;
 import com.google.cloud.ServiceRpc;
@@ -46,6 +49,9 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
   public static final String PROJECT_ID_ENV_VAR = "DATASTORE_PROJECT_ID";
   public static final String LOCAL_HOST_ENV_VAR = "DATASTORE_EMULATOR_HOST";
 
+  private transient TransportChannelProvider channelProvider = null;
+  private transient CredentialsProvider credentialsProvider = null;
+
   private final String namespace;
   private final String databaseId;
 
@@ -69,6 +75,10 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
         if (options.getTransportOptions() instanceof GrpcTransportOptions) {
           return new GrpcDatastoreRpc(options);
         } else if (options.getTransportOptions() instanceof HttpTransportOptions) {
+          // todo see if we can remove this check
+          if (DatastoreUtils.isEmulator(options)) {
+            throw new IllegalArgumentException("Only GRPC channels are allowed for emulator.");
+          }
           return new HttpDatastoreRpc(options);
         } else {
           throw new IllegalArgumentException(
@@ -84,13 +94,17 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
 
     private String namespace;
     private String databaseId;
+    private TransportChannelProvider channelProvider = null;
+    private CredentialsProvider credentialsProvider = null;
 
     private Builder() {}
 
     private Builder(DatastoreOptions options) {
       super(options);
-      namespace = options.namespace;
-      databaseId = options.databaseId;
+      this.namespace = options.namespace;
+      this.databaseId = options.databaseId;
+      this.channelProvider = validateChannelProvider(options.channelProvider);
+      this.credentialsProvider = options.credentialsProvider;
     }
 
     @Override
@@ -98,6 +112,28 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
       return super.setTransportOptions(transportOptions);
     }
 
+    /**
+     * Sets the {@link TransportChannelProvider} to use with this Datastore client.
+     *
+     * @param channelProvider A InstantiatingGrpcChannelProvider object that defines the transport
+     *     provider for this client.
+     */
+    public Builder setChannelProvider(TransportChannelProvider channelProvider) {
+      this.channelProvider = validateChannelProvider(channelProvider);
+      return this;
+    }
+
+    /**
+     * Sets the {@link CredentialsProvider} to use with this Datastore client.
+     *
+     * @param credentialsProvider A CredentialsProvider object that defines the credential provider
+     *     for this client.
+     */
+    public Builder setCredentialsProvider(CredentialsProvider credentialsProvider) {
+      this.credentialsProvider = credentialsProvider;
+      return this;
+    }
+
     @Override
     public DatastoreOptions build() {
       return new DatastoreOptions(this);
@@ -115,10 +151,45 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
     }
   }
 
+  private static TransportChannelProvider validateChannelProvider(
+      TransportChannelProvider channelProvider) {
+    if (!(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());
     namespace = MoreObjects.firstNonNull(builder.namespace, defaultNamespace());
     databaseId = MoreObjects.firstNonNull(builder.databaseId, DEFAULT_DATABASE_ID);
+
+    // todo see if we can update this
+    if (getTransportOptions() instanceof HttpTransportOptions
+        && (builder.channelProvider != null || builder.credentialsProvider != null)) {
+      throw new IllegalArgumentException(
+          "Only gRPC transport allows setting of channel provider or credentials provider");
+    } else if (getTransportOptions() instanceof GrpcTransportOptions) {
+      this.channelProvider =
+          builder.channelProvider != null
+              ? builder.channelProvider
+              : GrpcTransportOptions.setUpChannelProvider(
+                  DatastoreSettings.defaultGrpcTransportProviderBuilder(), this);
+
+      this.credentialsProvider =
+          builder.credentialsProvider != null
+              ? builder.credentialsProvider
+              : GrpcTransportOptions.setUpCredentialsProvider(this);
+    }
+  }
+
+  public CredentialsProvider getCredentialsProvider() {
+    return credentialsProvider;
+  }
+
+  public TransportChannelProvider getTransportChannelProvider() {
+    return channelProvider;
   }
 
   @Override
@@ -134,6 +205,7 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
   }
 
   private static class DatastoreDefaults implements ServiceDefaults<Datastore, DatastoreOptions> {
+    private final TransportOptions TRANSPORT_OPTIONS = getDefaultTransportOptionsBuilder().build();
 
     @Override
     public DatastoreFactory getDefaultServiceFactory() {
@@ -147,7 +219,11 @@ public class DatastoreOptions extends ServiceOptions<Datastore, DatastoreOptions
 
     @Override
     public TransportOptions getDefaultTransportOptions() {
-      return getDefaultGrpcTransportOptions();
+      return TRANSPORT_OPTIONS;
+    }
+
+    public static GrpcTransportOptions.Builder getDefaultTransportOptionsBuilder() {
+      return GrpcTransportOptions.newBuilder();
     }
   }
 
diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java
index ae1c7e07..7ca92872 100644
--- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java
+++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java
@@ -17,6 +17,7 @@
 package com.google.cloud.datastore;
 
 import com.google.api.core.InternalApi;
+import com.google.cloud.NoCredentials;
 import com.google.common.base.Strings;
 import java.net.InetAddress;
 import java.net.URL;
@@ -24,6 +25,11 @@ import java.net.URL;
 @InternalApi
 public class DatastoreUtils {
 
+  public static boolean isEmulator(DatastoreOptions datastoreOptions) {
+    return isLocalHost(datastoreOptions.getHost())
+        || NoCredentials.getInstance().equals(datastoreOptions.getCredentials());
+  }
+
   public static boolean isLocalHost(String host) {
     if (Strings.isNullOrEmpty(host)) {
       return false;
diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java
index ffcab056..3d9aebc6 100644
--- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java
+++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java
@@ -16,7 +16,7 @@
 
 package com.google.cloud.datastore.spi.v1;
 
-import static com.google.cloud.datastore.DatastoreUtils.isLocalHost;
+import static com.google.cloud.datastore.DatastoreUtils.isEmulator;
 import static com.google.cloud.datastore.DatastoreUtils.removeScheme;
 import static com.google.cloud.datastore.spi.v1.RpcUtils.retrySettingSetter;
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -30,14 +30,12 @@ import com.google.api.gax.rpc.ClientContext;
 import com.google.api.gax.rpc.HeaderProvider;
 import com.google.api.gax.rpc.NoHeaderProvider;
 import com.google.api.gax.rpc.TransportChannel;
-import com.google.cloud.NoCredentials;
 import com.google.cloud.ServiceOptions;
 import com.google.cloud.datastore.DatastoreException;
 import com.google.cloud.datastore.DatastoreOptions;
 import com.google.cloud.datastore.v1.DatastoreSettings;
 import com.google.cloud.datastore.v1.stub.DatastoreStubSettings;
 import com.google.cloud.datastore.v1.stub.GrpcDatastoreStub;
-import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.common.base.Strings;
 import com.google.datastore.v1.AllocateIdsRequest;
 import com.google.datastore.v1.AllocateIdsResponse;
@@ -69,7 +67,6 @@ public class GrpcDatastoreRpc implements DatastoreRpc {
   private boolean closed;
 
   public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException {
-
     try {
       clientContext =
           isEmulator(datastoreOptions)
@@ -146,11 +143,6 @@ public class GrpcDatastoreRpc implements DatastoreRpc {
     return closed && datastoreStub.isShutdown();
   }
 
-  private boolean isEmulator(DatastoreOptions datastoreOptions) {
-    return isLocalHost(datastoreOptions.getHost())
-        || NoCredentials.getInstance().equals(datastoreOptions.getCredentials());
-  }
-
   private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOptions)
       throws IOException {
     ManagedChannel managedChannel =
@@ -177,11 +169,8 @@ public class GrpcDatastoreRpc implements DatastoreRpc {
 
     DatastoreSettingsBuilder settingsBuilder =
         new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build());
-    settingsBuilder.setCredentialsProvider(
-        GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions));
-    settingsBuilder.setTransportChannelProvider(
-        GrpcTransportOptions.setUpChannelProvider(
-            DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions));
+    settingsBuilder.setCredentialsProvider(datastoreOptions.getCredentialsProvider());
+    settingsBuilder.setTransportChannelProvider(datastoreOptions.getTransportChannelProvider());
     settingsBuilder.setInternalHeaderProvider(internalHeaderProvider);
     settingsBuilder.setHeaderProvider(
         datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider()));
diff --git a/pom.xml b/pom.xml
index 73ac365f..c71e85d9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -14,7 +14,7 @@
   <parent>
     <groupId>com.google.cloud</groupId>
     <artifactId>google-cloud-shared-config</artifactId>
-    <version>1.6.1</version>
+    <version>1.7.1</version>
   </parent>
 
   <developers>
@@ -143,7 +143,7 @@
     <github.global.server>github</github.global.server>
     <site.installationModule>google-cloud-datastore-parent</site.installationModule>
     <project.javadoc.protobufBaseURL>https://googleapis.dev/java/google-api-grpc/latest</project.javadoc.protobufBaseURL>
-    <error-prone.version>2.23.0</error-prone.version>
+    <error-prone.version>2.24.1</error-prone.version>
   </properties>
 
   <dependencyManagement>
@@ -151,7 +151,7 @@
       <dependency>
         <groupId>com.google.cloud</groupId>
         <artifactId>google-cloud-shared-dependencies</artifactId>
-        <version>3.19.0</version>
+        <version>3.24.0</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>
@@ -237,7 +237,7 @@
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-compiler-plugin</artifactId>
-            <version>3.11.0</version>
+            <version>3.12.1</version>
             <configuration>
               <encoding>UTF-8</encoding>
               <fork>true</fork>


Test patch:
diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/BlobTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/BlobTest.java
index 55d5f031..391f6f76 100644
--- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/BlobTest.java
+++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/BlobTest.java
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 
+import com.google.common.testing.EqualsTester;
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
@@ -46,7 +47,8 @@ public class BlobTest {
 
   @Test
   public void testEquals() {
-    assertEquals(blob1, blob1);
+    EqualsTester equalsTester = new EqualsTester();
+    equalsTester.addEqualityGroup(blob1, blob1).testEquals();
     assertEquals(blob1, Blob.copyFrom(bytes1));
     assertNotEquals(blob1, blob2);
   }
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 6281befe..098ae342 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
@@ -22,11 +22,17 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
+import com.google.api.gax.core.NoCredentialsProvider;
+import com.google.api.gax.grpc.ChannelPoolSettings;
+import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
+import com.google.cloud.NoCredentials;
 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 org.easymock.EasyMock;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -48,7 +54,9 @@ public class DatastoreOptionsTest {
             .setServiceRpcFactory(datastoreRpcFactory)
             .setProjectId(PROJECT_ID)
             .setDatabaseId(DATABASE_ID)
+            .setCredentials(NoCredentials.getInstance())
             .setHost("http://localhost:" + PORT);
+
     EasyMock.expect(datastoreRpcFactory.create(EasyMock.anyObject(DatastoreOptions.class)))
         .andReturn(datastoreRpc)
         .anyTimes();
@@ -81,6 +89,51 @@ public class DatastoreOptionsTest {
     assertSame(datastoreRpc, options.build().getRpc());
   }
 
+  @Test
+  public void testCustomChannelAndCredentials() {
+    NoCredentialsProvider noCredentialsProvider = NoCredentialsProvider.create();
+    InstantiatingGrpcChannelProvider channelProvider =
+        DatastoreSettings.defaultGrpcTransportProviderBuilder()
+            .setChannelPoolSettings(
+                ChannelPoolSettings.builder()
+                    .setInitialChannelCount(10)
+                    .setMaxChannelCount(20)
+                    .build())
+            .build();
+    DatastoreOptions datastoreOptions =
+        DatastoreOptions.newBuilder()
+            .setServiceRpcFactory(datastoreRpcFactory)
+            .setProjectId(PROJECT_ID)
+            .setDatabaseId(DATABASE_ID)
+            .setChannelProvider(channelProvider)
+            .setCredentialsProvider(noCredentialsProvider)
+            .setHost("http://localhost:" + PORT)
+            .build();
+    assertEquals(datastoreOptions.getTransportChannelProvider(), channelProvider);
+    assertEquals(datastoreOptions.getCredentialsProvider(), noCredentialsProvider);
+  }
+
+  @Test
+  public void testInvalidConfigForHttp() {
+    DatastoreOptions.Builder options =
+        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())
+            .setCredentialsProvider(NoCredentialsProvider.create())
+            .setHost("http://localhost:" + PORT);
+    Assert.assertThrows(IllegalArgumentException.class, options::build);
+  }
+
   @Test
   public void testTransport() {
     // default grpc transport
@@ -93,6 +146,8 @@ public class DatastoreOptionsTest {
             .setProjectId(PROJECT_ID)
             .build();
     assertThat(httpDatastoreOptions.getTransportOptions()).isInstanceOf(HttpTransportOptions.class);
+    assertThat(httpDatastoreOptions.getCredentialsProvider()).isNull();
+    assertThat(httpDatastoreOptions.getTransportChannelProvider()).isNull();
 
     // custom grpc transport
     DatastoreOptions grpcDatastoreOptions =
diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/LatLngTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/LatLngTest.java
index e8076eba..9363fc15 100644
--- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/LatLngTest.java
+++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/LatLngTest.java
@@ -19,6 +19,7 @@ package com.google.cloud.datastore;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 
+import com.google.common.testing.EqualsTester;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -40,7 +41,8 @@ public class LatLngTest {
 
   @Test
   public void testEquals() {
-    assertEquals(gp1, gp1);
+    EqualsTester equalsTester = new EqualsTester();
+    equalsTester.addEqualityGroup(gp1, gp1).testEquals();
     assertNotEquals(gp1, gp2);
   }