From 556a2c54e654538c1b0e7f44e5c3f7e679caf632 Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Wed, 23 Apr 2025 16:58:56 +0530 Subject: [PATCH 1/8] feat: support option for setting client-id --- .../com/google/cloud/spanner/Spanner.java | 23 +++++++++++ .../com/google/cloud/spanner/SpannerImpl.java | 5 ++- .../spanner/connection/ConnectionImpl.java | 39 +++++++++++++++---- .../spanner/connection/ConnectionOptions.java | 7 +++- .../connection/ConnectionProperties.java | 8 ++++ .../google/cloud/spanner/SpannerImplTest.java | 34 ++++++++++++++++ 6 files changed, 106 insertions(+), 10 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java index 7ccbc88d978..f4b6cda894f 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java @@ -125,6 +125,29 @@ public interface Spanner extends Service, AutoCloseable { */ DatabaseClient getDatabaseClient(DatabaseId db); + + /** + * Returns a {@code DatabaseClient} for the given database and given client id. It uses a pool of sessions to talk to + * the database. + * + * + *
{@code
+   * SpannerOptions options = SpannerOptions.newBuilder().build();
+   * Spanner spanner = options.getService();
+   * final String project = "test-project";
+   * final String instance = "test-instance";
+   * final String database = "example-db";
+   * final String client_id = "client_id"
+   * DatabaseId db =
+   *     DatabaseId.of(project, instance, database);
+   *
+   * DatabaseClient dbClient = spanner.getDatabaseClient(db, client_id);
+   * }
+ * + * + */ + DatabaseClient getDatabaseClient(DatabaseId db, String clientId); + /** * Returns a {@code BatchClient} to do batch operations on Cloud Spanner databases. Batch client * is useful when one wants to read/query a large amount of data from Cloud Spanner across diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 63d501fbe63..dc1392480ea 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -254,9 +254,12 @@ public InstanceAdminClient getInstanceAdminClient() { @Override public DatabaseClient getDatabaseClient(DatabaseId db) { + return getDatabaseClient(db, null); + } + + public DatabaseClient getDatabaseClient(DatabaseId db, String clientId) { synchronized (this) { checkClosed(); - String clientId = null; if (dbClients.containsKey(db) && !dbClients.get(db).isValid()) { // Close the invalidated client and remove it. dbClients.get(db).closeAsync(new ClosedException()); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index c1e8839534f..482be295df9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -108,6 +108,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.Stack; import java.util.UUID; @@ -157,6 +158,7 @@ class ConnectionImpl implements Connection { private static final ParsedStatement RELEASE_STATEMENT = AbstractStatementParser.getInstance(Dialect.GOOGLE_STANDARD_SQL) .parse(Statement.of("RELEASE s1")); + private static final String CLIENT_ID = "client_id"; /** * Exception that is used to register the stacktrace of the code that opened a {@link Connection}. @@ -251,8 +253,8 @@ static UnitOfWorkType of(TransactionMode transactionMode) { } } - private StatementExecutor.StatementTimeout statementTimeout = - new StatementExecutor.StatementTimeout(); + private StatementTimeout statementTimeout = + new StatementTimeout(); private boolean closed = false; private final Spanner spanner; @@ -323,7 +325,28 @@ static UnitOfWorkType of(TransactionMode transactionMode) { EmulatorUtil.maybeCreateInstanceAndDatabase( spanner, options.getDatabaseId(), options.getDialect()); } - this.dbClient = spanner.getDatabaseClient(options.getDatabaseId()); + DatabaseClient tempDbClient = null; + try { + final Map> propertyValueMap = options.getInitialConnectionPropertyValues(); + String clientIdString = null; + if (propertyValueMap != null) { + final ConnectionPropertyValue clientIdProp = propertyValueMap.get(CLIENT_ID); + if (clientIdProp != null) { + Object value = clientIdProp.getValue(); + if (value != null) { + clientIdString = value.toString(); + } + } + if (clientIdString != null && !clientIdString.isEmpty()) { + tempDbClient = spanner.getDatabaseClient(options.getDatabaseId(), clientIdString); + } + } else { + tempDbClient = spanner.getDatabaseClient(options.getDatabaseId()); + } + } catch(Exception e) { + tempDbClient = spanner.getDatabaseClient(options.getDatabaseId()); + } + this.dbClient = tempDbClient; this.batchClient = spanner.getBatchClient(options.getDatabaseId()); this.ddlClient = createDdlClient(); this.connectionState = @@ -411,7 +434,7 @@ static Attributes createOpenTelemetryAttributes(DatabaseId databaseId) { } @VisibleForTesting - ConnectionState.Type getConnectionStateType() { + Type getConnectionStateType() { return this.connectionState.getType(); } @@ -500,7 +523,7 @@ private void reset(Context context, boolean inTransaction) { this.connectionState.resetValue(AUTOCOMMIT_DML_MODE, context, inTransaction); this.statementTag = null; - this.statementTimeout = new StatementExecutor.StatementTimeout(); + this.statementTimeout = new StatementTimeout(); this.connectionState.resetValue(DIRECTED_READ, context, inTransaction); this.connectionState.resetValue(SAVEPOINT_SUPPORT, context, inTransaction); this.protoDescriptors = null; @@ -542,7 +565,7 @@ public boolean isClosed() { } private T getConnectionPropertyValue( - com.google.cloud.spanner.connection.ConnectionProperty property) { + ConnectionProperty property) { return this.connectionState.getValue(property).getValue(); } @@ -563,8 +586,8 @@ private void setConnectionPropertyValue( * Sets a connection property value only for the duration of the current transaction. The effects * of this will be undone once the transaction ends, regardless whether the transaction is * committed or rolled back. 'Local' properties are supported for both {@link - * com.google.cloud.spanner.connection.ConnectionState.Type#TRANSACTIONAL} and {@link - * com.google.cloud.spanner.connection.ConnectionState.Type#NON_TRANSACTIONAL} connection states. + * Type#TRANSACTIONAL} and {@link + * Type#NON_TRANSACTIONAL} connection states. * *

NOTE: This feature is not yet exposed in the public API. */ diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index 6f945938df0..a09c6872753 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -46,6 +46,7 @@ import static com.google.cloud.spanner.connection.ConnectionProperties.RETURN_COMMIT_STATS; import static com.google.cloud.spanner.connection.ConnectionProperties.ROUTE_TO_LEADER; import static com.google.cloud.spanner.connection.ConnectionProperties.TRACING_PREFIX; +import static com.google.cloud.spanner.connection.ConnectionProperties.CLIENT_ID; import static com.google.cloud.spanner.connection.ConnectionProperties.TRACK_CONNECTION_LEAKS; import static com.google.cloud.spanner.connection.ConnectionProperties.TRACK_SESSION_LEAKS; import static com.google.cloud.spanner.connection.ConnectionProperties.USER_AGENT; @@ -539,6 +540,11 @@ public Builder setTracingPrefix(String tracingPrefix) { return this; } + public Builder setClientId(String clientId) { + setConnectionPropertyValue(CLIENT_ID, clientId); + return this; + } + /** @return the {@link ConnectionOptions} */ public ConnectionOptions build() { Preconditions.checkState(this.uri != null, "Connection URI is required"); @@ -603,7 +609,6 @@ private ConnectionOptions(Builder builder) { // Create the initial connection state from the parsed properties in the connection URL. this.initialConnectionState = new ConnectionState(connectionPropertyValues); - // Check that at most one of credentials location, encoded credentials, credentials provider and // OUAuth token has been specified in the connection URI. Preconditions.checkArgument( diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java index 54d3461b787..22f541f3bd2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java @@ -135,6 +135,14 @@ public class ConnectionProperties { private static final Boolean[] BOOLEANS = new Boolean[] {Boolean.TRUE, Boolean.FALSE}; + static final ConnectionProperty CLIENT_ID = + create( + "client_id", + "Client Id to use for this connection. Can only be set at the start up time", + null, + StringValueConverter.INSTANCE, + Context.STARTUP); + static final ConnectionProperty CONNECTION_STATE_TYPE = create( "connection_state_type", diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index 3cf13dc58d3..609d8b98365 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -358,6 +358,40 @@ public void testCreateInstanceAdminClient_whenMockAdminSettings_assertException( assertNotNull(instanceAdminClient); } + @Test + public void testGetDatabaseClient_when_clientId_is_not_null() { + String dbName = + String.format("projects/p1/instances/i1/databases/%s", UUID.randomUUID().toString()); + DatabaseId db = DatabaseId.of(dbName); + + Mockito.when(spannerOptions.getTransportOptions()) + .thenReturn(GrpcTransportOptions.newBuilder().build()); + Mockito.when(spannerOptions.getSessionPoolOptions()) + .thenReturn(SessionPoolOptions.newBuilder().setMinSessions(0).build()); + Mockito.when(spannerOptions.getDatabaseRole()).thenReturn("role"); + + DatabaseClientImpl databaseClient = (DatabaseClientImpl) impl.getDatabaseClient(db, "clientId-1"); + assertThat(databaseClient.clientId).isEqualTo("clientId-1"); + + // Get same db client again. + DatabaseClientImpl databaseClient1 = (DatabaseClientImpl) impl.getDatabaseClient(db, "clientId-1"); + assertThat(databaseClient1.clientId).isEqualTo(databaseClient.clientId); + + // Get a db client for a different database. + String dbName2 = + String.format("projects/p1/instances/i1/databases/%s", UUID.randomUUID().toString()); + DatabaseId db2 = DatabaseId.of(dbName2); + DatabaseClientImpl databaseClient2 = (DatabaseClientImpl) impl.getDatabaseClient(db2, "clientId-1"); + assertThat(databaseClient2.clientId).isEqualTo("clientId-1"); + + // Getting a new database client for an invalidated database should use the same client id. + databaseClient.pool.setResourceNotFoundException( + new DatabaseNotFoundException(DoNotConstructDirectly.ALLOWED, "not found", null, null)); + DatabaseClientImpl revalidated = (DatabaseClientImpl) impl.getDatabaseClient(db, "clientId-1"); + assertThat(revalidated).isNotSameInstanceAs(databaseClient); + assertThat(revalidated.clientId).isEqualTo(databaseClient.clientId); + } + private void closeSpannerAndIncludeStacktrace(Spanner spanner) { spanner.close(); } From d814755daf86e44c38e92af1e97dee5e929301e3 Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Wed, 23 Apr 2025 18:00:27 +0530 Subject: [PATCH 2/8] feat: support option for setting client-id --- .../com/google/cloud/spanner/connection/ConnectionImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index 482be295df9..90c2b164cb7 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -340,6 +340,9 @@ static UnitOfWorkType of(TransactionMode transactionMode) { if (clientIdString != null && !clientIdString.isEmpty()) { tempDbClient = spanner.getDatabaseClient(options.getDatabaseId(), clientIdString); } + else { + tempDbClient = spanner.getDatabaseClient(options.getDatabaseId()); + } } else { tempDbClient = spanner.getDatabaseClient(options.getDatabaseId()); } From 3c587df30dba573017b7c73351c13152a5621798 Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Wed, 23 Apr 2025 18:13:45 +0530 Subject: [PATCH 3/8] feat: support option for setting client-id --- .../spanner/connection/ConnectionImpl.java | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index 90c2b164cb7..c60b665be91 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -109,6 +109,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.Stack; import java.util.UUID; @@ -326,28 +327,18 @@ static UnitOfWorkType of(TransactionMode transactionMode) { spanner, options.getDatabaseId(), options.getDialect()); } DatabaseClient tempDbClient = null; + final DatabaseId databaseId = options.getDatabaseId(); try { - final Map> propertyValueMap = options.getInitialConnectionPropertyValues(); - String clientIdString = null; - if (propertyValueMap != null) { - final ConnectionPropertyValue clientIdProp = propertyValueMap.get(CLIENT_ID); - if (clientIdProp != null) { - Object value = clientIdProp.getValue(); - if (value != null) { - clientIdString = value.toString(); - } - } - if (clientIdString != null && !clientIdString.isEmpty()) { - tempDbClient = spanner.getDatabaseClient(options.getDatabaseId(), clientIdString); - } - else { - tempDbClient = spanner.getDatabaseClient(options.getDatabaseId()); - } - } else { - tempDbClient = spanner.getDatabaseClient(options.getDatabaseId()); + Optional clientIdOpt = extractClientIdOptional(options); + if(clientIdOpt.isPresent()) { + tempDbClient = spanner.getDatabaseClient(databaseId, clientIdOpt.get()); } } catch(Exception e) { - tempDbClient = spanner.getDatabaseClient(options.getDatabaseId()); + System.err.println("WARNING: Failed during DatabaseClient initialization (possibly getting specific ID), falling back to default. Error: " + + e.getMessage()); + } + if(tempDbClient == null) { + tempDbClient = spanner.getDatabaseClient(databaseId); } this.dbClient = tempDbClient; this.batchClient = spanner.getBatchClient(options.getDatabaseId()); @@ -366,6 +357,14 @@ && getDialect() == Dialect.POSTGRESQL setDefaultTransactionOptions(getDefaultIsolationLevel()); } + private Optional extractClientIdOptional(ConnectionOptions options) { + return Optional.ofNullable(options.getInitialConnectionPropertyValues()) + .map(props -> props.get(CLIENT_ID)) + .map(ConnectionPropertyValue::getValue) + .map(Object::toString) + .filter(id -> !id.isEmpty()); + } + /** Constructor only for test purposes. */ @VisibleForTesting ConnectionImpl( From 27c1d9598900d21e5bbb73a090fef5c77a93d0ff Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Wed, 23 Apr 2025 18:18:07 +0530 Subject: [PATCH 4/8] feat: support option for setting client-id --- .../com/google/cloud/spanner/Spanner.java | 5 ++-- .../spanner/connection/ConnectionImpl.java | 23 ++++++++----------- .../spanner/connection/ConnectionOptions.java | 2 +- .../google/cloud/spanner/SpannerImplTest.java | 9 +++++--- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java index f4b6cda894f..54a7f91d4a9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java @@ -125,10 +125,9 @@ public interface Spanner extends Service, AutoCloseable { */ DatabaseClient getDatabaseClient(DatabaseId db); - /** - * Returns a {@code DatabaseClient} for the given database and given client id. It uses a pool of sessions to talk to - * the database. + * Returns a {@code DatabaseClient} for the given database and given client id. It uses a pool of + * sessions to talk to the database. * * *

{@code
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java
index c60b665be91..dc03b68dbb7 100644
--- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java
+++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java
@@ -108,7 +108,6 @@
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.Stack;
@@ -254,8 +253,7 @@ static UnitOfWorkType of(TransactionMode transactionMode) {
     }
   }
 
-  private StatementTimeout statementTimeout =
-      new StatementTimeout();
+  private StatementTimeout statementTimeout = new StatementTimeout();
   private boolean closed = false;
 
   private final Spanner spanner;
@@ -330,14 +328,15 @@ static UnitOfWorkType of(TransactionMode transactionMode) {
     final DatabaseId databaseId = options.getDatabaseId();
     try {
       Optional clientIdOpt = extractClientIdOptional(options);
-      if(clientIdOpt.isPresent()) {
+      if (clientIdOpt.isPresent()) {
         tempDbClient = spanner.getDatabaseClient(databaseId, clientIdOpt.get());
       }
-    } catch(Exception e) {
-      System.err.println("WARNING: Failed during DatabaseClient initialization (possibly getting specific ID), falling back to default. Error: "
-          + e.getMessage());
+    } catch (Exception e) {
+      System.err.println(
+          "WARNING: Failed during DatabaseClient initialization (possibly getting specific ID), falling back to default. Error: "
+              + e.getMessage());
     }
-    if(tempDbClient == null) {
+    if (tempDbClient == null) {
       tempDbClient = spanner.getDatabaseClient(databaseId);
     }
     this.dbClient = tempDbClient;
@@ -566,8 +565,7 @@ public boolean isClosed() {
     return closed;
   }
 
-  private  T getConnectionPropertyValue(
-      ConnectionProperty property) {
+  private  T getConnectionPropertyValue(ConnectionProperty property) {
     return this.connectionState.getValue(property).getValue();
   }
 
@@ -587,9 +585,8 @@ private  void setConnectionPropertyValue(
   /**
    * Sets a connection property value only for the duration of the current transaction. The effects
    * of this will be undone once the transaction ends, regardless whether the transaction is
-   * committed or rolled back. 'Local' properties are supported for both {@link
-   * Type#TRANSACTIONAL} and {@link
-   * Type#NON_TRANSACTIONAL} connection states.
+   * committed or rolled back. 'Local' properties are supported for both {@link Type#TRANSACTIONAL}
+   * and {@link Type#NON_TRANSACTIONAL} connection states.
    *
    * 

NOTE: This feature is not yet exposed in the public API. */ diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index a09c6872753..fbc59caa174 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -21,6 +21,7 @@ import static com.google.cloud.spanner.connection.ConnectionProperties.AUTO_PARTITION_MODE; import static com.google.cloud.spanner.connection.ConnectionProperties.CHANNEL_PROVIDER; import static com.google.cloud.spanner.connection.ConnectionProperties.CLIENT_CERTIFICATE; +import static com.google.cloud.spanner.connection.ConnectionProperties.CLIENT_ID; import static com.google.cloud.spanner.connection.ConnectionProperties.CLIENT_KEY; import static com.google.cloud.spanner.connection.ConnectionProperties.CREDENTIALS_PROVIDER; import static com.google.cloud.spanner.connection.ConnectionProperties.CREDENTIALS_URL; @@ -46,7 +47,6 @@ import static com.google.cloud.spanner.connection.ConnectionProperties.RETURN_COMMIT_STATS; import static com.google.cloud.spanner.connection.ConnectionProperties.ROUTE_TO_LEADER; import static com.google.cloud.spanner.connection.ConnectionProperties.TRACING_PREFIX; -import static com.google.cloud.spanner.connection.ConnectionProperties.CLIENT_ID; import static com.google.cloud.spanner.connection.ConnectionProperties.TRACK_CONNECTION_LEAKS; import static com.google.cloud.spanner.connection.ConnectionProperties.TRACK_SESSION_LEAKS; import static com.google.cloud.spanner.connection.ConnectionProperties.USER_AGENT; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index 609d8b98365..af73b2b7e5f 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -370,18 +370,21 @@ public void testGetDatabaseClient_when_clientId_is_not_null() { .thenReturn(SessionPoolOptions.newBuilder().setMinSessions(0).build()); Mockito.when(spannerOptions.getDatabaseRole()).thenReturn("role"); - DatabaseClientImpl databaseClient = (DatabaseClientImpl) impl.getDatabaseClient(db, "clientId-1"); + DatabaseClientImpl databaseClient = + (DatabaseClientImpl) impl.getDatabaseClient(db, "clientId-1"); assertThat(databaseClient.clientId).isEqualTo("clientId-1"); // Get same db client again. - DatabaseClientImpl databaseClient1 = (DatabaseClientImpl) impl.getDatabaseClient(db, "clientId-1"); + DatabaseClientImpl databaseClient1 = + (DatabaseClientImpl) impl.getDatabaseClient(db, "clientId-1"); assertThat(databaseClient1.clientId).isEqualTo(databaseClient.clientId); // Get a db client for a different database. String dbName2 = String.format("projects/p1/instances/i1/databases/%s", UUID.randomUUID().toString()); DatabaseId db2 = DatabaseId.of(dbName2); - DatabaseClientImpl databaseClient2 = (DatabaseClientImpl) impl.getDatabaseClient(db2, "clientId-1"); + DatabaseClientImpl databaseClient2 = + (DatabaseClientImpl) impl.getDatabaseClient(db2, "clientId-1"); assertThat(databaseClient2.clientId).isEqualTo("clientId-1"); // Getting a new database client for an invalidated database should use the same client id. From c770b77e2962762ea9432c301b9842ec12260b2f Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Wed, 23 Apr 2025 18:56:10 +0530 Subject: [PATCH 5/8] feat: support option for setting client-id --- google-cloud-spanner/clirr-ignored-differences.xml | 7 +++++++ .../src/main/java/com/google/cloud/spanner/Spanner.java | 4 +++- .../main/java/com/google/cloud/spanner/SpannerImpl.java | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index d6d36b0e147..2d934fb1b2b 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -957,4 +957,11 @@ com/google/cloud/spanner/DatabaseClient com.google.cloud.spanner.Statement$StatementFactory getStatementFactory() + + + 7012 + com/google/cloud/spanner/Spanner + com.google.cloud.spanner.DatabaseClient getDatabaseClient(com.google.cloud.spanner.DatabaseId, java.lang.String) + Added as default method for JDBC client ID support. Default implementation throws UnsupportedOperationException to maintain binary compatibility for existing implementers. + diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java index 54a7f91d4a9..1fa21d9f870 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java @@ -145,7 +145,9 @@ public interface Spanner extends Service, AutoCloseable { * * */ - DatabaseClient getDatabaseClient(DatabaseId db, String clientId); + default DatabaseClient getDatabaseClient(DatabaseId db, String clientId) { + throw new UnsupportedOperationException("getDatabaseClient with clientId is not supported by this default implementation."); + } /** * Returns a {@code BatchClient} to do batch operations on Cloud Spanner databases. Batch client diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index dc1392480ea..6aad072c97a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -257,6 +257,7 @@ public DatabaseClient getDatabaseClient(DatabaseId db) { return getDatabaseClient(db, null); } + @Override public DatabaseClient getDatabaseClient(DatabaseId db, String clientId) { synchronized (this) { checkClosed(); From b2079f4064dcce1b977450806929130e9e3d3d55 Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Thu, 24 Apr 2025 13:06:54 +0530 Subject: [PATCH 6/8] feat: support option for setting client-id --- .../src/main/java/com/google/cloud/spanner/Spanner.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java index 1fa21d9f870..587a95ace63 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java @@ -146,7 +146,8 @@ public interface Spanner extends Service, AutoCloseable { * */ default DatabaseClient getDatabaseClient(DatabaseId db, String clientId) { - throw new UnsupportedOperationException("getDatabaseClient with clientId is not supported by this default implementation."); + throw new UnsupportedOperationException( + "getDatabaseClient with clientId is not supported by this default implementation."); } /** From a2f790a274bd1c3d4f4291ddfb1f79a213a6c91b Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Thu, 24 Apr 2025 16:45:44 +0530 Subject: [PATCH 7/8] feat: support option for setting client-id --- .../clirr-ignored-differences.xml | 7 ----- .../google/cloud/spanner/ExtendedSpanner.java | 28 +++++++++++++++++++ .../com/google/cloud/spanner/Spanner.java | 25 ----------------- .../com/google/cloud/spanner/SpannerImpl.java | 2 +- .../spanner/connection/ConnectionImpl.java | 8 ++++-- 5 files changed, 35 insertions(+), 35 deletions(-) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index 83ad125f7e1..28e22e4a86c 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -964,11 +964,4 @@ com/google/cloud/spanner/DatabaseClient com.google.cloud.spanner.Statement$StatementFactory getStatementFactory() - - - 7012 - com/google/cloud/spanner/Spanner - com.google.cloud.spanner.DatabaseClient getDatabaseClient(com.google.cloud.spanner.DatabaseId, java.lang.String) - Added as default method for JDBC client ID support. Default implementation throws UnsupportedOperationException to maintain binary compatibility for existing implementers. - diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java new file mode 100644 index 00000000000..58f80c86e2f --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java @@ -0,0 +1,28 @@ +package com.google.cloud.spanner; + +public interface ExtendedSpanner extends Spanner { + /** + * Returns a {@code DatabaseClient} for the given database and given client id. It uses a pool of + * sessions to talk to the database. + * + * + *

{@code
+   * SpannerOptions options = SpannerOptions.newBuilder().build();
+   * Spanner spanner = options.getService();
+   * final String project = "test-project";
+   * final String instance = "test-instance";
+   * final String database = "example-db";
+   * final String client_id = "client_id"
+   * DatabaseId db =
+   *     DatabaseId.of(project, instance, database);
+   *
+   * DatabaseClient dbClient = spanner.getDatabaseClient(db, client_id);
+   * }
+ * + * + */ + default DatabaseClient getDatabaseClient(DatabaseId db, String clientId) { + throw new UnsupportedOperationException( + "getDatabaseClient with clientId is not supported by this default implementation."); + } +} diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java index 587a95ace63..7ccbc88d978 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java @@ -125,31 +125,6 @@ public interface Spanner extends Service, AutoCloseable { */ DatabaseClient getDatabaseClient(DatabaseId db); - /** - * Returns a {@code DatabaseClient} for the given database and given client id. It uses a pool of - * sessions to talk to the database. - * - * - *
{@code
-   * SpannerOptions options = SpannerOptions.newBuilder().build();
-   * Spanner spanner = options.getService();
-   * final String project = "test-project";
-   * final String instance = "test-instance";
-   * final String database = "example-db";
-   * final String client_id = "client_id"
-   * DatabaseId db =
-   *     DatabaseId.of(project, instance, database);
-   *
-   * DatabaseClient dbClient = spanner.getDatabaseClient(db, client_id);
-   * }
- * - * - */ - default DatabaseClient getDatabaseClient(DatabaseId db, String clientId) { - throw new UnsupportedOperationException( - "getDatabaseClient with clientId is not supported by this default implementation."); - } - /** * Returns a {@code BatchClient} to do batch operations on Cloud Spanner databases. Batch client * is useful when one wants to read/query a large amount of data from Cloud Spanner across diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 6aad072c97a..c4b11cc859d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -59,7 +59,7 @@ import javax.annotation.concurrent.GuardedBy; /** Default implementation of the Cloud Spanner interface. */ -class SpannerImpl extends BaseService implements Spanner { +class SpannerImpl extends BaseService implements ExtendedSpanner { private static final Logger logger = Logger.getLogger(SpannerImpl.class.getName()); final TraceWrapper tracer = new TraceWrapper( diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index dc03b68dbb7..0c73ba11ffb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -58,6 +58,7 @@ import com.google.cloud.spanner.DatabaseId; import com.google.cloud.spanner.Dialect; import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.ExtendedSpanner; import com.google.cloud.spanner.Mutation; import com.google.cloud.spanner.Options; import com.google.cloud.spanner.Options.QueryOption; @@ -328,8 +329,11 @@ static UnitOfWorkType of(TransactionMode transactionMode) { final DatabaseId databaseId = options.getDatabaseId(); try { Optional clientIdOpt = extractClientIdOptional(options); - if (clientIdOpt.isPresent()) { - tempDbClient = spanner.getDatabaseClient(databaseId, clientIdOpt.get()); + if (clientIdOpt.isPresent() && !clientIdOpt.get().isEmpty()) { + if (this.spanner instanceof ExtendedSpanner) { + ExtendedSpanner extendedSpanner = (ExtendedSpanner) this.spanner; + tempDbClient = extendedSpanner.getDatabaseClient(databaseId, clientIdOpt.get()); + } } } catch (Exception e) { System.err.println( From c78ef3d4de70df5a60a4e5129169ba15ddaa5b8c Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Thu, 24 Apr 2025 16:56:09 +0530 Subject: [PATCH 8/8] feat: support option for setting client-id --- .../google/cloud/spanner/ExtendedSpanner.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java index 58f80c86e2f..a1b65ecfefb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ExtendedSpanner.java @@ -1,3 +1,19 @@ +/* + * Copyright 2025 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.google.cloud.spanner; public interface ExtendedSpanner extends Spanner {