From edc5bbf0d9d4faf48fd9a8d479d5bc5de938c82d Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 6 Feb 2023 13:29:38 +0530 Subject: [PATCH 01/20] fix: prevent illegal negative timeout values into thread sleep() method while retrying exceptions in unit tests. * For details on issue see - https://github.com/googleapis/java-spanner/issues/2206 --- .../com/google/cloud/spanner/it/ITClosedSessionTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java index aeb0256285..227611a10d 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java @@ -251,7 +251,10 @@ public void testTransactionManager() throws InterruptedException { break; } } catch (AbortedException e) { - Thread.sleep(e.getRetryDelayInMillis()); + long retryDelayInMillis = e.getRetryDelayInMillis(); + if(retryDelayInMillis > 0) { + Thread.sleep(retryDelayInMillis); + } txn = manager.resetForRetry(); } } From 4cd497b05eab3e3b6b89b582bfafde80d42c1518 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Wed, 8 Feb 2023 15:27:18 +0530 Subject: [PATCH 02/20] Fixing lint issues. --- .../java/com/google/cloud/spanner/it/ITClosedSessionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java index 227611a10d..efbffcfa89 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITClosedSessionTest.java @@ -252,7 +252,7 @@ public void testTransactionManager() throws InterruptedException { } } catch (AbortedException e) { long retryDelayInMillis = e.getRetryDelayInMillis(); - if(retryDelayInMillis > 0) { + if (retryDelayInMillis > 0) { Thread.sleep(retryDelayInMillis); } txn = manager.resetForRetry(); From 5d090eade38d9ddb0a48943a26b8cbcbe58f4709 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Fri, 9 Feb 2024 18:46:04 +0530 Subject: [PATCH 03/20] feat: support emulator with autogenerated admin clients. --- .../com/google/cloud/spanner/Spanner.java | 19 +++++++ .../com/google/cloud/spanner/SpannerImpl.java | 52 +++++++++++++++++++ 2 files changed, 71 insertions(+) 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 52c35cb713..6cba16f19c 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 @@ -18,6 +18,8 @@ import com.google.api.gax.core.ExecutorProvider; import com.google.cloud.Service; +import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; +import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings; /** * An interface for Cloud Spanner. Typically, there would only be one instance of this for the @@ -38,6 +40,14 @@ public interface Spanner extends Service, AutoCloseable { */ DatabaseAdminClient getDatabaseAdminClient(); + /** + * + * @param settings + * @return + */ + com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient getDatabaseAdminClient( + DatabaseAdminSettings... settings); + /** Returns an {@code InstanceAdminClient} to do admin operations on Cloud Spanner instances. */ /* * @@ -50,6 +60,15 @@ public interface Spanner extends Service, AutoCloseable { */ InstanceAdminClient getInstanceAdminClient(); + /** + * + * @param settings + * @return + */ + com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient getInstanceAdminClient( + InstanceAdminSettings... settings); + + /** * Returns a {@code DatabaseClient} for the given database. It uses a pool of sessions to talk to * the database. 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 5ff916bbe9..b0bb665ade 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 @@ -25,6 +25,14 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.SessionClient.SessionId; import com.google.cloud.spanner.SpannerOptions.CloseableExecutorProvider; +import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.database.v1.stub.GrpcDatabaseAdminStub; +import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.GrpcInstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; @@ -40,6 +48,7 @@ import io.opencensus.trace.Tracer; import io.opencensus.trace.Tracing; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -191,11 +200,54 @@ public DatabaseAdminClient getDatabaseAdminClient() { return dbAdminClient; } + @Override + public com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient getDatabaseAdminClient( + DatabaseAdminSettings... settings) { + if(settings.length != 0) { + com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient client = + com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( + Arrays.stream(settings).iterator().next()); + return client; + } else { + DatabaseAdminStub databaseAdminStub = + GrpcDatabaseAdminStub.create( + getOptions() + .getDatabaseAdminStubSettings() + .toBuilder() + .setTransportChannelProvider(channelProvider) + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .build()); + return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create(databaseAdminStub); + } + } + @Override public InstanceAdminClient getInstanceAdminClient() { return instanceClient; } + @Override + public com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient getInstanceAdminClient( + InstanceAdminSettings... settings) { + if(settings.length != 0) { + com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient client = + com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create(Arrays.stream(settings).iterator().next()); + return client; + } else { + InstanceAdminStub instanceAdminStub = + GrpcInstanceAdminStub.create( + getOptions() + .getInstanceAdminStubSettings() + .toBuilder() + .setTransportChannelProvider(channelProvider) + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .build()); + return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create(instanceAdminStub); + } + } + @Override public DatabaseClient getDatabaseClient(DatabaseId db) { synchronized (this) { From 5c9c2e4e6008b8db2e0f97f6912b141597bb6e90 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Sat, 10 Feb 2024 20:52:00 +0530 Subject: [PATCH 04/20] chore: modifying public interfaces and adding an integration test. --- .../com/google/cloud/spanner/Spanner.java | 10 +- .../com/google/cloud/spanner/SpannerImpl.java | 47 +--- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 10 + .../cloud/spanner/spi/v1/SpannerRpc.java | 17 ++ .../it/ITAutogeneratedAdminClientTest.java | 254 ++++++++++++++++++ 5 files changed, 290 insertions(+), 48 deletions(-) create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java 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 6cba16f19c..083987bc8c 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 @@ -18,8 +18,6 @@ import com.google.api.gax.core.ExecutorProvider; import com.google.cloud.Service; -import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; -import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings; /** * An interface for Cloud Spanner. Typically, there would only be one instance of this for the @@ -42,11 +40,9 @@ public interface Spanner extends Service, AutoCloseable { /** * - * @param settings * @return */ - com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient getDatabaseAdminClient( - DatabaseAdminSettings... settings); + com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient(); /** Returns an {@code InstanceAdminClient} to do admin operations on Cloud Spanner instances. */ /* @@ -62,11 +58,9 @@ com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient getDatabaseAdminC /** * - * @param settings * @return */ - com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient getInstanceAdminClient( - InstanceAdminSettings... settings); + com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient(); /** 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 b0bb665ade..b04fd056d6 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 @@ -27,12 +27,9 @@ import com.google.cloud.spanner.SpannerOptions.CloseableExecutorProvider; import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; -import com.google.cloud.spanner.admin.database.v1.stub.GrpcDatabaseAdminStub; import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings; import com.google.cloud.spanner.admin.instance.v1.stub.GrpcInstanceAdminStub; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; @@ -47,6 +44,7 @@ import io.opencensus.metrics.LabelValue; import io.opencensus.trace.Tracer; import io.opencensus.trace.Tracing; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -201,25 +199,9 @@ public DatabaseAdminClient getDatabaseAdminClient() { } @Override - public com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient getDatabaseAdminClient( - DatabaseAdminSettings... settings) { - if(settings.length != 0) { - com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient client = - com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( - Arrays.stream(settings).iterator().next()); - return client; - } else { - DatabaseAdminStub databaseAdminStub = - GrpcDatabaseAdminStub.create( - getOptions() - .getDatabaseAdminStubSettings() - .toBuilder() - .setTransportChannelProvider(channelProvider) - .setCredentialsProvider(credentialsProvider) - .setStreamWatchdogProvider(watchdogProvider) - .build()); - return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create(databaseAdminStub); - } + public com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() { + DatabaseAdminStub databaseAdminStub = gapicRpc.getDatabaseAdminStub(); + return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create(databaseAdminStub); } @Override @@ -228,24 +210,9 @@ public InstanceAdminClient getInstanceAdminClient() { } @Override - public com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient getInstanceAdminClient( - InstanceAdminSettings... settings) { - if(settings.length != 0) { - com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient client = - com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create(Arrays.stream(settings).iterator().next()); - return client; - } else { - InstanceAdminStub instanceAdminStub = - GrpcInstanceAdminStub.create( - getOptions() - .getInstanceAdminStubSettings() - .toBuilder() - .setTransportChannelProvider(channelProvider) - .setCredentialsProvider(credentialsProvider) - .setStreamWatchdogProvider(watchdogProvider) - .build()); - return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create(instanceAdminStub); - } + public com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() { + InstanceAdminStub instanceAdminStub = gapicRpc.getInstanceAdminStub(); + return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create(instanceAdminStub); } @Override diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index d75b6636a5..18c219ea8c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -2058,4 +2058,14 @@ private static Duration systemProperty(String name, int defaultValue) { String stringValue = System.getProperty(name, ""); return Duration.ofSeconds(stringValue.isEmpty() ? defaultValue : Integer.parseInt(stringValue)); } + + @Override + public DatabaseAdminStub getDatabaseAdminStub() { + return databaseAdminStub; + } + + @Override + public InstanceAdminStub getInstanceAdminStub() { + return instanceAdminStub; + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java index 39f798e011..aab7bcb036 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java @@ -497,4 +497,21 @@ TestIamPermissionsResponse testInstanceAdminIAMPermissions( void shutdown(); boolean isClosed(); + + /** + * + * @return + */ + default InstanceAdminStub getInstanceAdminStub() { + throw new UnsupportedOperationException("Not implemented"); + } + + /** + * + * @return + */ + default DatabaseAdminStub getDatabaseAdminStub() { + throw new UnsupportedOperationException("Not implemented"); + } + } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java new file mode 100644 index 0000000000..1be4ce01af --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java @@ -0,0 +1,254 @@ +/* + * Copyright 2022 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.it; + +import static com.google.cloud.spanner.testing.EmulatorSpannerHelper.isUsingEmulator; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.*; +import static org.junit.Assume.assumeFalse; + +import com.google.api.gax.rpc.PermissionDeniedException; +import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.InstanceId; +import com.google.cloud.spanner.IntegrationTestEnv; +import com.google.cloud.spanner.ParallelIntegrationTest; +import com.google.cloud.spanner.ResultSet; +import com.google.cloud.spanner.Spanner; +import com.google.cloud.spanner.SpannerException; +import com.google.cloud.spanner.SpannerOptions; +import com.google.cloud.spanner.Statement; +import com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient; +import com.google.cloud.spanner.testing.RemoteSpannerHelper; +import com.google.common.collect.ImmutableList; +import com.google.spanner.admin.database.v1.CreateDatabaseRequest; +import com.google.spanner.admin.database.v1.Database; +import com.google.spanner.admin.database.v1.DatabaseDialect; +import com.google.spanner.admin.database.v1.DatabaseName; +import com.google.spanner.admin.database.v1.InstanceName; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +/** + * Integration tests for Role Permissions using {@link com.google.cloud.spanner.DatabaseRole}. + */ +@Category(ParallelIntegrationTest.class) +@RunWith(Parameterized.class) +public class ITAutogeneratedAdminClientTest { + + @ClassRule + public static IntegrationTestEnv env = new IntegrationTestEnv(); + private static DatabaseAdminClient dbAdminClient; + private static RemoteSpannerHelper testHelper; + + private static List databasesToDrop; + + @Parameter + public DatabaseDialect dialect; + + @Parameters(name = "Dialect = {0}") + public static List data() { + return ImmutableList.of( + DatabaseDialect.GOOGLE_STANDARD_SQL, DatabaseDialect.POSTGRESQL); + } + + @BeforeClass + public static void setUp() { + assumeFalse("Emulator does not support database roles", isUsingEmulator()); + testHelper = env.getTestHelper(); + dbAdminClient = testHelper.getClient().databaseAdminClient(); + databasesToDrop = new ArrayList<>(); + } + + @AfterClass + public static void cleanup() throws Exception { + if (databasesToDrop != null) { + for (DatabaseName databaseName : databasesToDrop) { + try { + dbAdminClient.dropDatabase(databaseName); + } catch (Exception e) { + System.err.println("Failed to drop database " + databaseName + ", skipping...: " + e.getMessage()); + } + } + } + } + + @Test + public void grantAndRevokeDatabaseRolePermissions() throws Exception { + // Create database with table and role permission. + final String dbRoleParent = "parent"; + final String databaseId = testHelper.getUniqueDatabaseId(); + final InstanceId instanceId = testHelper.getInstanceId(); + + final String createTableT = getCreateTableStatement(); + final String createRoleParent = String.format("CREATE ROLE %s", dbRoleParent); + final String grantSelectOnTableToParent = + dialect == DatabaseDialect.POSTGRESQL + ? String.format("GRANT SELECT ON TABLE T TO %s", dbRoleParent) + : String.format("GRANT SELECT ON TABLE T TO ROLE %s", dbRoleParent); + final Database createdDatabase = + createAndUpdateDatabase( + testHelper.getOptions().getProjectId(), + instanceId, + databaseId, + ImmutableList.of(createTableT, createRoleParent, grantSelectOnTableToParent)); + + // Connect to db with dbRoleParent. + SpannerOptions options = + testHelper.getOptions().toBuilder().setDatabaseRole(dbRoleParent).build(); + + Spanner spanner = options.getService(); + DatabaseClient dbClient = spanner.getDatabaseClient(createdDatabase.getName()); + + // Test SELECT permissions to role dbRoleParent on table T. + // Query using dbRoleParent should return result. + try (ResultSet rs = + dbClient.singleUse().executeQuery(Statement.of("SELECT COUNT(*) as cnt FROM T"))) { + assertTrue(rs.next()); + assertEquals(dbClient.getDatabaseRole(), dbRoleParent); + } catch (PermissionDeniedException e) { + // This is not expected + fail("Got PermissionDeniedException when it should not have occurred."); + } + + // Revoke select Permission for dbRoleParent. + final String revokeSelectOnTableFromParent = + dialect == DatabaseDialect.POSTGRESQL + ? String.format("REVOKE SELECT ON TABLE T FROM %s", dbRoleParent) + : String.format("REVOKE SELECT ON TABLE T FROM ROLE %s", dbRoleParent); + + dbAdminClient.updateDatabaseDdlAsync( + DatabaseName.of(options.getProjectId(), instanceId.getInstance(), databaseId), + ImmutableList.of(revokeSelectOnTableFromParent)) + .get(5, TimeUnit.MINUTES); + + // Test SELECT permissions to role dbRoleParent on table T. + // Query using dbRoleParent should return PermissionDeniedException. + try (ResultSet rs = + dbClient.singleUse().executeQuery(Statement.of("SELECT COUNT(*) as cnt FROM T"))) { + SpannerException e = assertThrows(SpannerException.class, () -> rs.next()); + assertThat(e.getErrorCode()).isEqualTo(ErrorCode.PERMISSION_DENIED); + assertThat(e.getMessage()).contains(dbRoleParent); + } + // Drop role and table. + final String dropTableT = "DROP TABLE T"; + final String dropRoleParent = String.format("DROP ROLE %s", dbRoleParent); + dbAdminClient.updateDatabaseDdlAsync( + DatabaseName.of(options.getProjectId(), instanceId.getInstance(), databaseId), + ImmutableList.of(dropTableT, dropRoleParent)) + .get(5, TimeUnit.MINUTES); + databasesToDrop.add(DatabaseName.parse(createdDatabase.getName())); + } + + @Test + public void roleWithNoPermissions() throws Exception { + final String dbRoleOrphan = testHelper.getUniqueDatabaseRole(); + final String databaseId = testHelper.getUniqueDatabaseId(); + final InstanceId instanceId = testHelper.getInstanceId(); + + final String createTableT = getCreateTableStatement(); + final String createRoleOrphan = String.format("CREATE ROLE %s", dbRoleOrphan); + + final Database createdDatabase = + createAndUpdateDatabase(testHelper.getOptions().getProjectId(), + instanceId, databaseId, ImmutableList.of(createTableT, createRoleOrphan)); + + // Connect to db with dbRoleOrphan + SpannerOptions options = + testHelper.getOptions().toBuilder().setDatabaseRole(dbRoleOrphan).build(); + + Spanner spanner = options.getService(); + DatabaseClient dbClient = spanner.getDatabaseClient(createdDatabase.getName()); + + // Test SELECT permissions to role dbRoleOrphan on table T. + // Query using dbRoleOrphan should return PermissionDeniedException. + try (ResultSet rs = + dbClient.singleUse().executeQuery(Statement.of("SELECT COUNT(*) as cnt FROM T"))) { + SpannerException e = assertThrows(SpannerException.class, () -> rs.next()); + assertThat(e.getErrorCode()).isEqualTo(ErrorCode.PERMISSION_DENIED); + assertThat(e.getMessage()).contains(dbRoleOrphan); + } + // Drop role and table. + final String dropTableT = "DROP TABLE T"; + final String dropRoleParent = String.format("DROP ROLE %s", dbRoleOrphan); + dbAdminClient.updateDatabaseDdlAsync( + DatabaseName.of(options.getProjectId(), instanceId.getInstance(), databaseId), + ImmutableList.of(dropTableT, dropRoleParent)) + .get(5, TimeUnit.MINUTES); + + databasesToDrop.add(DatabaseName.parse(createdDatabase.getName())); + } + + private Database createAndUpdateDatabase( + String projectId, final InstanceId instanceId, final String databaseId, final List statements) + throws Exception { + if (dialect == DatabaseDialect.POSTGRESQL) { + // DDL statements other than are not allowed in database creation request + // for PostgreSQL-enabled databases. + CreateDatabaseRequest createDatabaseRequest = + CreateDatabaseRequest.newBuilder() + .setParent(InstanceName.of(projectId, instanceId.getInstance()).toString()) + .setCreateStatement(getCreateDatabaseStatement(databaseId, dialect)) + .setDatabaseDialect(dialect).build(); + Database database = dbAdminClient + .createDatabaseAsync(createDatabaseRequest) + .get(10, TimeUnit.MINUTES); + dbAdminClient + .updateDatabaseDdlAsync(database.getName(), statements).get(5, TimeUnit.MINUTES); + return database; + } else { + CreateDatabaseRequest createDatabaseRequest = + CreateDatabaseRequest.newBuilder() + .setParent(InstanceName.of(projectId, instanceId.getInstance()).toString()) + .setCreateStatement(getCreateDatabaseStatement(databaseId, dialect)) + .setDatabaseDialect(dialect) + .addAllExtraStatements(statements).build(); + return dbAdminClient + .createDatabaseAsync(createDatabaseRequest) + .get(10, TimeUnit.MINUTES); + + } + } + + private String getCreateTableStatement() { + if (dialect == DatabaseDialect.POSTGRESQL) { + return "CREATE TABLE T (" + " \"K\" VARCHAR PRIMARY KEY" + ")"; + } else { + return "CREATE TABLE T (" + " K STRING(MAX)" + ") PRIMARY KEY (K)"; + } + } + + static String getCreateDatabaseStatement( + final String databaseName, final DatabaseDialect dialect) { + if (dialect == DatabaseDialect.GOOGLE_STANDARD_SQL) { + return "CREATE DATABASE `" + databaseName + "`"; + } else { + return "CREATE DATABASE \"" + databaseName + "\""; + } + } +} + From bd4156bf6998ae900ebccbc8aff163880612607c Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Sun, 11 Feb 2024 01:32:11 +0530 Subject: [PATCH 05/20] chore: add deprecated annotations and add docs. --- .../com/google/cloud/spanner/Spanner.java | 44 ++++++++-- .../com/google/cloud/spanner/SpannerImpl.java | 5 -- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 20 ++--- .../cloud/spanner/spi/v1/SpannerRpc.java | 9 +- .../it/ITAutogeneratedAdminClientTest.java | 87 ++++++++++++------- 5 files changed, 113 insertions(+), 52 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 083987bc8c..e5444e6084 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 @@ -26,7 +26,13 @@ * quota. */ public interface Spanner extends Service, AutoCloseable { - /** Returns a {@code DatabaseAdminClient} to do admin operations on Cloud Spanner databases. */ + + /** + * Returns a {@code DatabaseAdminClient} to do admin operations on Cloud Spanner databases. + * + * @return + * @deprecated Use {@link #databaseAdminClient()} instead. + */ /* * * 
{@code
@@ -36,15 +42,32 @@ public interface Spanner extends Service, AutoCloseable {
    * }
* */ + @Deprecated DatabaseAdminClient getDatabaseAdminClient(); /** + * Returns a {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} to do admin + * operations on Cloud Spanner databases. * - * @return + * @return {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} + */ + /* + * + * 
{@code
+   * SpannerOptions options = SpannerOptions.newBuilder().build();
+   * Spanner spanner = options.getService();
+   * DatabaseAdminClient dbAdminClient = spanner.databaseAdminClient();
+   * }
+ * */ com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient(); - /** Returns an {@code InstanceAdminClient} to do admin operations on Cloud Spanner instances. */ + /** + * Returns an {@code InstanceAdminClient} to do admin operations on Cloud Spanner instances. + * + * @return {@code InstanceAdminClient} + * @deprecated Use {@link #instanceAdminClient()}} instead. + */ /* * * 
{@code
@@ -54,15 +77,26 @@ public interface Spanner extends Service, AutoCloseable {
    * }
* */ + @Deprecated InstanceAdminClient getInstanceAdminClient(); /** + * Returns an {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} to do admin + * operations on Cloud Spanner instances. * - * @return + * @return {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} + */ + /* + * + * 
{@code
+   * SpannerOptions options = SpannerOptions.newBuilder().build();
+   * Spanner spanner = options.getService();
+   * InstanceAdminClient instanceAdminClient = spanner.instanceAdminClient();
+   * }
+ * */ com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient(); - /** * Returns a {@code DatabaseClient} for the given database. It uses a pool of sessions to talk to * the database. 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 b04fd056d6..a83501b422 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 @@ -25,10 +25,7 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.SessionClient.SessionId; import com.google.cloud.spanner.SpannerOptions.CloseableExecutorProvider; -import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings; -import com.google.cloud.spanner.admin.instance.v1.stub.GrpcInstanceAdminStub; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; @@ -44,9 +41,7 @@ import io.opencensus.metrics.LabelValue; import io.opencensus.trace.Tracer; import io.opencensus.trace.Tracing; -import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 18c219ea8c..712205f1ae 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -1987,6 +1987,16 @@ public boolean isClosed() { return rpcIsClosed; } + @Override + public DatabaseAdminStub getDatabaseAdminStub() { + return databaseAdminStub; + } + + @Override + public InstanceAdminStub getInstanceAdminStub() { + return instanceAdminStub; + } + private static final class GrpcStreamingCall implements StreamingCall { private final ApiCallContext callContext; private final StreamController controller; @@ -2058,14 +2068,4 @@ private static Duration systemProperty(String name, int defaultValue) { String stringValue = System.getProperty(name, ""); return Duration.ofSeconds(stringValue.isEmpty() ? defaultValue : Integer.parseInt(stringValue)); } - - @Override - public DatabaseAdminStub getDatabaseAdminStub() { - return databaseAdminStub; - } - - @Override - public InstanceAdminStub getInstanceAdminStub() { - return instanceAdminStub; - } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java index aab7bcb036..b45cb9df24 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java @@ -499,19 +499,22 @@ TestIamPermissionsResponse testInstanceAdminIAMPermissions( boolean isClosed(); /** + * Getter method to obtain the auto-generated instance admin client stub. * - * @return + * @return InstanceAdminStub */ + @InternalApi default InstanceAdminStub getInstanceAdminStub() { throw new UnsupportedOperationException("Not implemented"); } /** + * Getter method to obtain the auto-generated database admin client stub. * - * @return + * @return DatabaseAdminStub */ + @InternalApi default DatabaseAdminStub getDatabaseAdminStub() { throw new UnsupportedOperationException("Not implemented"); } - } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java index 1be4ce01af..374e8e440e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2022 Google LLC + * Copyright 2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import com.google.api.gax.rpc.PermissionDeniedException; import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.DatabaseId; import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.InstanceId; import com.google.cloud.spanner.IntegrationTestEnv; @@ -33,13 +34,17 @@ import com.google.cloud.spanner.SpannerOptions; import com.google.cloud.spanner.Statement; import com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient; +import com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient; import com.google.cloud.spanner.testing.RemoteSpannerHelper; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterators; import com.google.spanner.admin.database.v1.CreateDatabaseRequest; import com.google.spanner.admin.database.v1.Database; import com.google.spanner.admin.database.v1.DatabaseDialect; import com.google.spanner.admin.database.v1.DatabaseName; import com.google.spanner.admin.database.v1.InstanceName; +import com.google.spanner.admin.instance.v1.InstanceConfig; +import com.google.spanner.admin.instance.v1.ProjectName; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -54,26 +59,27 @@ import org.junit.runners.Parameterized.Parameters; /** - * Integration tests for Role Permissions using {@link com.google.cloud.spanner.DatabaseRole}. + * Integration tests for testing the auto-generated database admin {@link + * com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} and instance admin clients {@link + * com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} */ @Category(ParallelIntegrationTest.class) @RunWith(Parameterized.class) public class ITAutogeneratedAdminClientTest { - @ClassRule - public static IntegrationTestEnv env = new IntegrationTestEnv(); + @ClassRule public static IntegrationTestEnv env = new IntegrationTestEnv(); private static DatabaseAdminClient dbAdminClient; + + private static InstanceAdminClient instanceAdminClient; private static RemoteSpannerHelper testHelper; private static List databasesToDrop; - @Parameter - public DatabaseDialect dialect; + @Parameter public DatabaseDialect dialect; @Parameters(name = "Dialect = {0}") public static List data() { - return ImmutableList.of( - DatabaseDialect.GOOGLE_STANDARD_SQL, DatabaseDialect.POSTGRESQL); + return ImmutableList.of(DatabaseDialect.GOOGLE_STANDARD_SQL, DatabaseDialect.POSTGRESQL); } @BeforeClass @@ -81,6 +87,7 @@ public static void setUp() { assumeFalse("Emulator does not support database roles", isUsingEmulator()); testHelper = env.getTestHelper(); dbAdminClient = testHelper.getClient().databaseAdminClient(); + instanceAdminClient = testHelper.getClient().instanceAdminClient(); databasesToDrop = new ArrayList<>(); } @@ -91,7 +98,8 @@ public static void cleanup() throws Exception { try { dbAdminClient.dropDatabase(databaseName); } catch (Exception e) { - System.err.println("Failed to drop database " + databaseName + ", skipping...: " + e.getMessage()); + System.err.println( + "Failed to drop database " + databaseName + ", skipping...: " + e.getMessage()); } } } @@ -122,7 +130,8 @@ public void grantAndRevokeDatabaseRolePermissions() throws Exception { testHelper.getOptions().toBuilder().setDatabaseRole(dbRoleParent).build(); Spanner spanner = options.getService(); - DatabaseClient dbClient = spanner.getDatabaseClient(createdDatabase.getName()); + DatabaseId id = DatabaseId.of(createdDatabase.getName()); + DatabaseClient dbClient = spanner.getDatabaseClient(id); // Test SELECT permissions to role dbRoleParent on table T. // Query using dbRoleParent should return result. @@ -141,7 +150,8 @@ public void grantAndRevokeDatabaseRolePermissions() throws Exception { ? String.format("REVOKE SELECT ON TABLE T FROM %s", dbRoleParent) : String.format("REVOKE SELECT ON TABLE T FROM ROLE %s", dbRoleParent); - dbAdminClient.updateDatabaseDdlAsync( + dbAdminClient + .updateDatabaseDdlAsync( DatabaseName.of(options.getProjectId(), instanceId.getInstance(), databaseId), ImmutableList.of(revokeSelectOnTableFromParent)) .get(5, TimeUnit.MINUTES); @@ -157,7 +167,8 @@ public void grantAndRevokeDatabaseRolePermissions() throws Exception { // Drop role and table. final String dropTableT = "DROP TABLE T"; final String dropRoleParent = String.format("DROP ROLE %s", dbRoleParent); - dbAdminClient.updateDatabaseDdlAsync( + dbAdminClient + .updateDatabaseDdlAsync( DatabaseName.of(options.getProjectId(), instanceId.getInstance(), databaseId), ImmutableList.of(dropTableT, dropRoleParent)) .get(5, TimeUnit.MINUTES); @@ -174,15 +185,19 @@ public void roleWithNoPermissions() throws Exception { final String createRoleOrphan = String.format("CREATE ROLE %s", dbRoleOrphan); final Database createdDatabase = - createAndUpdateDatabase(testHelper.getOptions().getProjectId(), - instanceId, databaseId, ImmutableList.of(createTableT, createRoleOrphan)); + createAndUpdateDatabase( + testHelper.getOptions().getProjectId(), + instanceId, + databaseId, + ImmutableList.of(createTableT, createRoleOrphan)); // Connect to db with dbRoleOrphan SpannerOptions options = testHelper.getOptions().toBuilder().setDatabaseRole(dbRoleOrphan).build(); Spanner spanner = options.getService(); - DatabaseClient dbClient = spanner.getDatabaseClient(createdDatabase.getName()); + DatabaseId id = DatabaseId.of(createdDatabase.getName()); + DatabaseClient dbClient = spanner.getDatabaseClient(id); // Test SELECT permissions to role dbRoleOrphan on table T. // Query using dbRoleOrphan should return PermissionDeniedException. @@ -195,7 +210,8 @@ public void roleWithNoPermissions() throws Exception { // Drop role and table. final String dropTableT = "DROP TABLE T"; final String dropRoleParent = String.format("DROP ROLE %s", dbRoleOrphan); - dbAdminClient.updateDatabaseDdlAsync( + dbAdminClient + .updateDatabaseDdlAsync( DatabaseName.of(options.getProjectId(), instanceId.getInstance(), databaseId), ImmutableList.of(dropTableT, dropRoleParent)) .get(5, TimeUnit.MINUTES); @@ -203,8 +219,25 @@ public void roleWithNoPermissions() throws Exception { databasesToDrop.add(DatabaseName.parse(createdDatabase.getName())); } + @Test + public void instanceConfigOperations() { + List configs = new ArrayList<>(); + Iterators.addAll( + configs, + instanceAdminClient + .listInstanceConfigs(ProjectName.of(testHelper.getOptions().getProjectId())) + .iterateAll() + .iterator()); + assertThat(configs.isEmpty()).isFalse(); + InstanceConfig config = instanceAdminClient.getInstanceConfig(configs.get(0).getName()); + assertThat(config.getName()).isEqualTo(configs.get(0).getName()); + } + private Database createAndUpdateDatabase( - String projectId, final InstanceId instanceId, final String databaseId, final List statements) + String projectId, + final InstanceId instanceId, + final String databaseId, + final List statements) throws Exception { if (dialect == DatabaseDialect.POSTGRESQL) { // DDL statements other than are not allowed in database creation request @@ -213,12 +246,11 @@ private Database createAndUpdateDatabase( CreateDatabaseRequest.newBuilder() .setParent(InstanceName.of(projectId, instanceId.getInstance()).toString()) .setCreateStatement(getCreateDatabaseStatement(databaseId, dialect)) - .setDatabaseDialect(dialect).build(); - Database database = dbAdminClient - .createDatabaseAsync(createDatabaseRequest) - .get(10, TimeUnit.MINUTES); - dbAdminClient - .updateDatabaseDdlAsync(database.getName(), statements).get(5, TimeUnit.MINUTES); + .setDatabaseDialect(dialect) + .build(); + Database database = + dbAdminClient.createDatabaseAsync(createDatabaseRequest).get(10, TimeUnit.MINUTES); + dbAdminClient.updateDatabaseDdlAsync(database.getName(), statements).get(5, TimeUnit.MINUTES); return database; } else { CreateDatabaseRequest createDatabaseRequest = @@ -226,11 +258,9 @@ private Database createAndUpdateDatabase( .setParent(InstanceName.of(projectId, instanceId.getInstance()).toString()) .setCreateStatement(getCreateDatabaseStatement(databaseId, dialect)) .setDatabaseDialect(dialect) - .addAllExtraStatements(statements).build(); - return dbAdminClient - .createDatabaseAsync(createDatabaseRequest) - .get(10, TimeUnit.MINUTES); - + .addAllExtraStatements(statements) + .build(); + return dbAdminClient.createDatabaseAsync(createDatabaseRequest).get(10, TimeUnit.MINUTES); } } @@ -251,4 +281,3 @@ static String getCreateDatabaseStatement( } } } - From 8b5d8002fd339fc5417cd4b336c9eaccc84ca2d0 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 10:51:28 +0530 Subject: [PATCH 06/20] chore: making interface methods default. --- .../src/main/java/com/google/cloud/spanner/Spanner.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 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 e5444e6084..350d169598 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 @@ -60,7 +60,9 @@ public interface Spanner extends Service, AutoCloseable { * } * */ - com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient(); + default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() { + throw new UnsupportedOperationException("Not implemented"); + } /** * Returns an {@code InstanceAdminClient} to do admin operations on Cloud Spanner instances. @@ -95,7 +97,9 @@ public interface Spanner extends Service, AutoCloseable { * } * */ - com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient(); + default com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() { + throw new UnsupportedOperationException("Not implemented"); + } /** * Returns a {@code DatabaseClient} for the given database. It uses a pool of sessions to talk to From 500fd02906f5051ae393229dada347ae340e026d Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 12:06:12 +0530 Subject: [PATCH 07/20] chore: add clirr ignore checks for public methods. --- .../clirr-ignored-differences.xml | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index 8482230632..cde280269a 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -461,4 +461,27 @@ com/google/cloud/spanner/Dialect java.lang.String getDefaultSchema() + + + 7012 + com/google/cloud/spanner/Spanner + com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() + + + 7012 + com/google/cloud/spanner/Spanner + com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() + + + + + 7012 + com/google/cloud/spanner/spi/v1/SpannerRpc + com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub getDatabaseAdminStub() + + + 7012 + com/google/cloud/spanner/spi/v1/SpannerRpc + com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub getInstanceAdminStub() + From 0be27193282a4763f6a9e9417ff0735a93b15756 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 14:21:09 +0530 Subject: [PATCH 08/20] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Knut Olav Løite --- .../src/main/java/com/google/cloud/spanner/Spanner.java | 2 +- 1 file changed, 1 insertion(+), 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 350d169598..bcc0a1f9bb 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 @@ -83,7 +83,7 @@ default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseA InstanceAdminClient getInstanceAdminClient(); /** - * Returns an {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} to do admin + * Returns an {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} to execute admin * operations on Cloud Spanner instances. * * @return {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} From ab05f88945770dfdb8b43b015b905f4b82c0fbf2 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 14:21:17 +0530 Subject: [PATCH 09/20] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Knut Olav Løite --- .../src/main/java/com/google/cloud/spanner/Spanner.java | 2 +- 1 file changed, 1 insertion(+), 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 bcc0a1f9bb..4832edec32 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 @@ -65,7 +65,7 @@ default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseA } /** - * Returns an {@code InstanceAdminClient} to do admin operations on Cloud Spanner instances. + * Returns an {@code InstanceAdminClient} to execute admin operations on Cloud Spanner instances. * * @return {@code InstanceAdminClient} * @deprecated Use {@link #instanceAdminClient()}} instead. From bb41cf7016c6b6264b1def0c46a8818541fdee33 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 14:21:33 +0530 Subject: [PATCH 10/20] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Knut Olav Løite --- .../src/main/java/com/google/cloud/spanner/Spanner.java | 2 +- 1 file changed, 1 insertion(+), 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 4832edec32..ec66983f5a 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 @@ -46,7 +46,7 @@ public interface Spanner extends Service, AutoCloseable { DatabaseAdminClient getDatabaseAdminClient(); /** - * Returns a {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} to do admin + * Returns a {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} to execute admin * operations on Cloud Spanner databases. * * @return {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} From 3e8caa23d266482ade133509bddacb07577954c8 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 14:21:58 +0530 Subject: [PATCH 11/20] Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Knut Olav Løite --- .../src/main/java/com/google/cloud/spanner/Spanner.java | 2 +- 1 file changed, 1 insertion(+), 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 ec66983f5a..41f9a5382d 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 @@ -28,7 +28,7 @@ public interface Spanner extends Service, AutoCloseable { /** - * Returns a {@code DatabaseAdminClient} to do admin operations on Cloud Spanner databases. + * Returns a {@code DatabaseAdminClient} to execute admin operations on Cloud Spanner databases. * * @return * @deprecated Use {@link #databaseAdminClient()} instead. From f71a4c1ad39b24174f080c83b443a111b2ab32d2 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 15:50:36 +0530 Subject: [PATCH 12/20] chore: address PR comments. --- .../com/google/cloud/spanner/Spanner.java | 6 +---- .../com/google/cloud/spanner/SpannerImpl.java | 25 ++++++++++++++++--- 2 files changed, 22 insertions(+), 9 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 41f9a5382d..a606e40b16 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 @@ -30,8 +30,7 @@ public interface Spanner extends Service, AutoCloseable { /** * Returns a {@code DatabaseAdminClient} to execute admin operations on Cloud Spanner databases. * - * @return - * @deprecated Use {@link #databaseAdminClient()} instead. + * @return {@code DatabaseAdminClient} */ /* * @@ -42,7 +41,6 @@ public interface Spanner extends Service, AutoCloseable { * } * */ - @Deprecated DatabaseAdminClient getDatabaseAdminClient(); /** @@ -68,7 +66,6 @@ default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseA * Returns an {@code InstanceAdminClient} to execute admin operations on Cloud Spanner instances. * * @return {@code InstanceAdminClient} - * @deprecated Use {@link #instanceAdminClient()}} instead. */ /* * @@ -79,7 +76,6 @@ default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseA * } * */ - @Deprecated InstanceAdminClient getInstanceAdminClient(); /** 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 20edbf569e..166dd5d4c0 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 @@ -101,6 +101,16 @@ private static String nextDatabaseClientId(DatabaseId databaseId) { @GuardedBy("this") private final Map dbClients = new HashMap<>(); + @GuardedBy("this") + private final Map< + InstanceAdminStub, com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient> + instanceAdminClients = new HashMap<>(); + + @GuardedBy("this") + private final Map< + DatabaseAdminStub, com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient> + databaseAdminClients = new HashMap<>(); + private final CloseableExecutorProvider asyncExecutorProvider; @GuardedBy("this") @@ -109,6 +119,9 @@ private static String nextDatabaseClientId(DatabaseId databaseId) { private final DatabaseAdminClient dbAdminClient; private final InstanceAdminClient instanceClient; + private final com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient; + private final com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient; + /** * Exception class used to track the stack trace at the point when a Spanner instance is closed. * This exception will be thrown if a user tries to use any resources that were returned by this @@ -137,6 +150,12 @@ static final class ClosedException extends RuntimeException { this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), gapicRpc); this.instanceClient = new InstanceAdminClientImpl(options.getProjectId(), gapicRpc, dbAdminClient); + this.databaseAdminClient = + com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( + gapicRpc.getDatabaseAdminStub()); + this.instanceAdminClient = + com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create( + gapicRpc.getInstanceAdminStub()); } SpannerImpl(SpannerOptions options) { @@ -211,8 +230,7 @@ public DatabaseAdminClient getDatabaseAdminClient() { @Override public com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() { - DatabaseAdminStub databaseAdminStub = gapicRpc.getDatabaseAdminStub(); - return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create(databaseAdminStub); + return databaseAdminClient; } @Override @@ -222,8 +240,7 @@ public InstanceAdminClient getInstanceAdminClient() { @Override public com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() { - InstanceAdminStub instanceAdminStub = gapicRpc.getInstanceAdminStub(); - return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create(instanceAdminStub); + return instanceAdminClient; } @Override From 83307189b2cefb9432cccbd1f1f9429234d8d326 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 17:15:35 +0530 Subject: [PATCH 13/20] chore: decouple client stub objects and rely on common stub settings object. --- .../clirr-ignored-differences.xml | 4 +-- .../com/google/cloud/spanner/Spanner.java | 8 ++--- .../com/google/cloud/spanner/SpannerImpl.java | 29 +++++++++++++++---- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 29 ++++++++++--------- .../cloud/spanner/spi/v1/SpannerRpc.java | 14 +++++---- 5 files changed, 52 insertions(+), 32 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index e5164f92c1..bae6d09eb1 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -555,12 +555,12 @@ 7012 com/google/cloud/spanner/spi/v1/SpannerRpc - com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub getDatabaseAdminStub() + com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings getDatabaseAdminStubSettings() 7012 com/google/cloud/spanner/spi/v1/SpannerRpc - com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub getInstanceAdminStub() + com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings getInstanceAdminStubSettings() 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 a606e40b16..b29fbabf73 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 @@ -44,8 +44,8 @@ public interface Spanner extends Service, AutoCloseable { DatabaseAdminClient getDatabaseAdminClient(); /** - * Returns a {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} to execute admin - * operations on Cloud Spanner databases. + * Returns a {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} to execute + * admin operations on Cloud Spanner databases. * * @return {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} */ @@ -79,8 +79,8 @@ default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseA InstanceAdminClient getInstanceAdminClient(); /** - * Returns an {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} to execute admin - * operations on Cloud Spanner instances. + * Returns an {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} to execute + * admin operations on Cloud Spanner instances. * * @return {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} */ 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 166dd5d4c0..d14799dbdb 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 @@ -42,6 +42,7 @@ import io.opencensus.trace.Tracing; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -150,12 +151,8 @@ static final class ClosedException extends RuntimeException { this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), gapicRpc); this.instanceClient = new InstanceAdminClientImpl(options.getProjectId(), gapicRpc, dbAdminClient); - this.databaseAdminClient = - com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( - gapicRpc.getDatabaseAdminStub()); - this.instanceAdminClient = - com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create( - gapicRpc.getInstanceAdminStub()); + this.databaseAdminClient = createDatabaseAdminClient(); + this.instanceAdminClient = createInstanceAdminClient(); } SpannerImpl(SpannerOptions options) { @@ -365,4 +362,24 @@ void setNextPageToken(String nextPageToken) { abstract S fromProto(T proto); } + + private com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient + createInstanceAdminClient() { + try { + return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create( + gapicRpc.getInstanceAdminStubSettings().createStub()); + } catch (IOException ex) { + throw SpannerExceptionFactory.newSpannerException(ex); + } + } + + private com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient + createDatabaseAdminClient() { + try { + return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( + gapicRpc.getDatabaseAdminStubSettings().createStub()); + } catch (IOException ex) { + throw SpannerExceptionFactory.newSpannerException(ex); + } + } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index a41f880046..0f4b227571 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -243,6 +243,7 @@ public class GapicSpannerRpc implements SpannerRpc { private final Set readRetryableCodes; private final SpannerStub partitionedDmlStub; private final RetrySettings partitionedDmlRetrySettings; + private final InstanceAdminStubSettings instanceAdminStubSettings; private final InstanceAdminStub instanceAdminStub; private final DatabaseAdminStubSettings databaseAdminStubSettings; private final DatabaseAdminStub databaseAdminStub; @@ -435,16 +436,15 @@ public GapicSpannerRpc(final SpannerOptions options) { .withCheckInterval(pdmlSettings.getStreamWatchdogCheckInterval())); } this.partitionedDmlStub = GrpcSpannerStub.create(pdmlSettings.build()); - - this.instanceAdminStub = - GrpcInstanceAdminStub.create( - options - .getInstanceAdminStubSettings() - .toBuilder() - .setTransportChannelProvider(channelProvider) - .setCredentialsProvider(credentialsProvider) - .setStreamWatchdogProvider(watchdogProvider) - .build()); + this.instanceAdminStubSettings = + options + .getInstanceAdminStubSettings() + .toBuilder() + .setTransportChannelProvider(channelProvider) + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .build(); + this.instanceAdminStub = GrpcInstanceAdminStub.create(instanceAdminStubSettings); this.databaseAdminStubSettings = options @@ -510,6 +510,7 @@ public UnaryCallable createUnaryCalla this.executeQueryRetryableCodes = null; this.partitionedDmlStub = null; this.databaseAdminStubSettings = null; + this.instanceAdminStubSettings = null; this.spannerWatchdog = null; this.partitionedDmlRetrySettings = null; } @@ -2005,13 +2006,13 @@ public boolean isClosed() { } @Override - public DatabaseAdminStub getDatabaseAdminStub() { - return databaseAdminStub; + public DatabaseAdminStubSettings getDatabaseAdminStubSettings() { + return databaseAdminStubSettings; } @Override - public InstanceAdminStub getInstanceAdminStub() { - return instanceAdminStub; + public InstanceAdminStubSettings getInstanceAdminStubSettings() { + return instanceAdminStubSettings; } private static final class GrpcStreamingCall implements StreamingCall { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java index 2aa3808c4f..7868f3ec09 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java @@ -28,7 +28,9 @@ import com.google.cloud.spanner.Restore; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.common.collect.ImmutableList; import com.google.iam.v1.GetPolicyOptions; @@ -502,22 +504,22 @@ TestIamPermissionsResponse testInstanceAdminIAMPermissions( boolean isClosed(); /** - * Getter method to obtain the auto-generated instance admin client stub. + * Getter method to obtain the auto-generated instance admin client stub settings. * - * @return InstanceAdminStub + * @return InstanceAdminStubSettings */ @InternalApi - default InstanceAdminStub getInstanceAdminStub() { + default InstanceAdminStubSettings getInstanceAdminStubSettings() { throw new UnsupportedOperationException("Not implemented"); } /** - * Getter method to obtain the auto-generated database admin client stub. + * Getter method to obtain the auto-generated database admin client stub settings. * - * @return DatabaseAdminStub + * @return DatabaseAdminStubSettings */ @InternalApi - default DatabaseAdminStub getDatabaseAdminStub() { + default DatabaseAdminStubSettings getDatabaseAdminStubSettings() { throw new UnsupportedOperationException("Not implemented"); } } From d7f70fa4101868db7d6049501e3ccdcc4345846d Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 17:20:21 +0530 Subject: [PATCH 14/20] chore: remove unused map objects. --- .../java/com/google/cloud/spanner/SpannerImpl.java | 12 ------------ 1 file changed, 12 deletions(-) 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 d14799dbdb..f48501be47 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 @@ -25,8 +25,6 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.SessionClient.SessionId; import com.google.cloud.spanner.SpannerOptions.CloseableExecutorProvider; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; @@ -102,16 +100,6 @@ private static String nextDatabaseClientId(DatabaseId databaseId) { @GuardedBy("this") private final Map dbClients = new HashMap<>(); - @GuardedBy("this") - private final Map< - InstanceAdminStub, com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient> - instanceAdminClients = new HashMap<>(); - - @GuardedBy("this") - private final Map< - DatabaseAdminStub, com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient> - databaseAdminClients = new HashMap<>(); - private final CloseableExecutorProvider asyncExecutorProvider; @GuardedBy("this") From 07c9cd1121f2cf1f6e51d89a8981944325b2ad7b Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 19:46:23 +0530 Subject: [PATCH 15/20] chore: fix broken unit tests. --- .../cloud/spanner/BatchClientImplTest.java | 15 +++++++++++- .../google/cloud/spanner/SessionImplTest.java | 15 +++++++++++- .../google/cloud/spanner/SpannerImplTest.java | 15 +++++++++++- .../spanner/TransactionManagerImplTest.java | 23 +++++++++++++++++-- .../spanner/TransactionRunnerImplTest.java | 17 +++++++++++++- 5 files changed, 79 insertions(+), 6 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java index b7c4834044..75eb899aaa 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java @@ -30,12 +30,17 @@ import com.google.cloud.Timestamp; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.protobuf.ByteString; import com.google.protobuf.util.Timestamps; import com.google.spanner.v1.Session; import com.google.spanner.v1.Transaction; import io.opentelemetry.api.OpenTelemetry; +import java.io.IOException; import java.util.Collections; import java.util.Map; import org.junit.Before; @@ -59,6 +64,10 @@ public final class BatchClientImplTest { @Mock private SpannerRpc gapicRpc; @Mock private SpannerOptions spannerOptions; + @Mock private InstanceAdminStubSettings instanceAdminStubSettings; + @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; + @Mock private DatabaseAdminStub databaseAdminStub; + @Mock private InstanceAdminStub instanceAdminStub; @Captor private ArgumentCaptor> optionsCaptor; @Mock private BatchTransactionId txnID; @@ -72,7 +81,7 @@ public static void setupOpenTelemetry() { @SuppressWarnings("unchecked") @Before - public void setUp() { + public void setUp() throws IOException { initMocks(this); DatabaseId db = DatabaseId.of(DB_NAME); when(spannerOptions.getNumChannels()).thenReturn(4); @@ -86,6 +95,10 @@ public void setUp() { GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); when(transportOptions.getExecutorFactory()).thenReturn(mock(ExecutorFactory.class)); when(spannerOptions.getTransportOptions()).thenReturn(transportOptions); + when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); + when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); + when(gapicRpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(gapicRpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); @SuppressWarnings("resource") SpannerImpl spanner = new SpannerImpl(gapicRpc, spannerOptions); client = new BatchClientImpl(spanner.getSessionClient(db)); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java index 87edb64134..59aa063d99 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java @@ -34,6 +34,10 @@ import com.google.cloud.Timestamp; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.protobuf.ByteString; @@ -52,6 +56,7 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Scope; +import java.io.IOException; import java.text.ParseException; import java.util.Calendar; import java.util.Collections; @@ -76,6 +81,10 @@ public class SessionImplTest { @Mock private SpannerRpc rpc; @Mock private SpannerOptions spannerOptions; + @Mock private InstanceAdminStubSettings instanceAdminStubSettings; + @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; + @Mock private DatabaseAdminStub databaseAdminStub; + @Mock private InstanceAdminStub instanceAdminStub; private com.google.cloud.spanner.Session session; @Captor private ArgumentCaptor> optionsCaptor; private Map options; @@ -88,7 +97,7 @@ public static void setupOpenTelemetry() { @SuppressWarnings("unchecked") @Before - public void setUp() { + public void setUp() throws IOException { MockitoAnnotations.initMocks(this); when(spannerOptions.getNumChannels()).thenReturn(4); when(spannerOptions.getPrefetchChunks()).thenReturn(1); @@ -101,6 +110,10 @@ public void setUp() { when(spannerOptions.getTransportOptions()).thenReturn(transportOptions); when(spannerOptions.getSessionPoolOptions()).thenReturn(mock(SessionPoolOptions.class)); when(spannerOptions.getOpenTelemetry()).thenReturn(OpenTelemetry.noop()); + when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); + when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); + when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); @SuppressWarnings("resource") SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); String dbName = "projects/p1/instances/i1/databases/d1"; 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 31a6cad4c8..dea2e3e320 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 @@ -29,9 +29,14 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.SpannerException.DoNotConstructDirectly; import com.google.cloud.spanner.SpannerImpl.ClosedException; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; import io.opentelemetry.api.OpenTelemetry; +import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; import java.util.Collections; @@ -55,6 +60,10 @@ public class SpannerImplTest { @Mock private SpannerRpc rpc; @Mock private SpannerOptions spannerOptions; + @Mock private InstanceAdminStubSettings instanceAdminStubSettings; + @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; + @Mock private DatabaseAdminStub databaseAdminStub; + @Mock private InstanceAdminStub instanceAdminStub; private SpannerImpl impl; @Captor ArgumentCaptor> options; @@ -66,7 +75,7 @@ public static void setupOpenTelemetry() { } @Before - public void setUp() { + public void setUp() throws IOException { MockitoAnnotations.initMocks(this); when(spannerOptions.getNumChannels()).thenReturn(4); when(spannerOptions.getDatabaseRole()).thenReturn("role"); @@ -75,6 +84,10 @@ public void setUp() { when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); when(spannerOptions.getSessionLabels()).thenReturn(Collections.emptyMap()); when(spannerOptions.getOpenTelemetry()).thenReturn(OpenTelemetry.noop()); + when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); + when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); + when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); impl = new SpannerImpl(rpc, spannerOptions); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java index dc28b333c4..fd6f57d484 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java @@ -34,6 +34,10 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; import com.google.cloud.spanner.TransactionManager.TransactionState; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.protobuf.ByteString; import com.google.protobuf.Empty; @@ -47,6 +51,7 @@ import com.google.spanner.v1.Session; import com.google.spanner.v1.Transaction; import io.opentelemetry.api.OpenTelemetry; +import java.io.IOException; import java.util.Collections; import java.util.UUID; import java.util.concurrent.Executors; @@ -77,6 +82,10 @@ public void release(ScheduledExecutorService exec) { @Mock private SessionImpl session; @Mock TransactionRunnerImpl.TransactionContextImpl txn; + @Mock private InstanceAdminStubSettings instanceAdminStubSettings; + @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; + @Mock private DatabaseAdminStub databaseAdminStub; + @Mock private InstanceAdminStub instanceAdminStub; private TransactionManagerImpl manager; @BeforeClass @@ -201,7 +210,7 @@ public void commitAfterRollbackFails() { @SuppressWarnings("unchecked") @Test - public void usesPreparedTransaction() { + public void usesPreparedTransaction() throws IOException { SpannerOptions options = mock(SpannerOptions.class); when(options.getNumChannels()).thenReturn(4); GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); @@ -248,6 +257,11 @@ public void usesPreparedTransaction() { com.google.protobuf.Timestamp.newBuilder() .setSeconds(System.currentTimeMillis() * 1000)) .build())); + when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); + when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); + when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); + DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); @@ -263,7 +277,7 @@ public void usesPreparedTransaction() { @SuppressWarnings({"unchecked", "resource"}) @Test - public void inlineBegin() { + public void inlineBegin() throws IOException { SpannerOptions options = mock(SpannerOptions.class); when(options.getNumChannels()).thenReturn(4); GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); @@ -332,6 +346,11 @@ public void inlineBegin() { com.google.protobuf.Timestamp.newBuilder() .setSeconds(System.currentTimeMillis() * 1000)) .build())); + when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); + when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); + when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); + DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java index cf5f901f65..db92e4caa2 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java @@ -34,6 +34,10 @@ import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; import com.google.cloud.spanner.SessionClient.SessionId; import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.common.base.Preconditions; import com.google.protobuf.ByteString; @@ -63,6 +67,7 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Scope; +import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.UUID; @@ -81,6 +86,7 @@ /** Unit test for {@link com.google.cloud.spanner.TransactionRunnerImpl} */ @RunWith(JUnit4.class) public class TransactionRunnerImplTest { + private static final class TestExecutorFactory implements ExecutorFactory { @Override @@ -97,6 +103,10 @@ public void release(ScheduledExecutorService exec) { @Mock private SpannerRpc rpc; @Mock private SessionImpl session; @Mock private TransactionRunnerImpl.TransactionContextImpl txn; + @Mock private InstanceAdminStubSettings instanceAdminStubSettings; + @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; + @Mock private DatabaseAdminStub databaseAdminStub; + @Mock private InstanceAdminStub instanceAdminStub; private TransactionRunnerImpl transactionRunner; private boolean firstRun; private boolean usedInlinedBegin; @@ -151,7 +161,7 @@ public void setUp() { @SuppressWarnings("unchecked") @Test - public void usesPreparedTransaction() { + public void usesPreparedTransaction() throws IOException { SpannerOptions options = mock(SpannerOptions.class); when(options.getNumChannels()).thenReturn(4); GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); @@ -196,6 +206,11 @@ public void usesPreparedTransaction() { .setCommitTimestamp( Timestamp.newBuilder().setSeconds(System.currentTimeMillis() * 1000)) .build())); + when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); + when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); + when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); + DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); From 447ec3a6ed94c7ce5021e2727412c0961d86dd0b Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Mon, 12 Feb 2024 20:23:03 +0530 Subject: [PATCH 16/20] chore: handle case when client is terminated/shutdown --- .../com/google/cloud/spanner/SpannerImpl.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) 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 f48501be47..f47dc36890 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 @@ -25,6 +25,8 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.SessionClient.SessionId; import com.google.cloud.spanner.SpannerOptions.CloseableExecutorProvider; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; @@ -108,8 +110,8 @@ private static String nextDatabaseClientId(DatabaseId databaseId) { private final DatabaseAdminClient dbAdminClient; private final InstanceAdminClient instanceClient; - private final com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient; - private final com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient; + private com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient; + private com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient; /** * Exception class used to track the stack trace at the point when a Spanner instance is closed. @@ -215,7 +217,10 @@ public DatabaseAdminClient getDatabaseAdminClient() { @Override public com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() { - return databaseAdminClient; + if(databaseAdminClient.isTerminated() || databaseAdminClient.isShutdown()) { + this.databaseAdminClient = createDatabaseAdminClient(); + } + return this.databaseAdminClient; } @Override @@ -225,7 +230,10 @@ public InstanceAdminClient getInstanceAdminClient() { @Override public com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() { - return instanceAdminClient; + if(instanceAdminClient.isTerminated() || instanceAdminClient.isShutdown()) { + this.instanceAdminClient = createInstanceAdminClient(); + } + return this.instanceAdminClient; } @Override @@ -354,8 +362,10 @@ void setNextPageToken(String nextPageToken) { private com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient createInstanceAdminClient() { try { + final InstanceAdminStubSettings settings = + Preconditions.checkNotNull(gapicRpc.getInstanceAdminStubSettings()); return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create( - gapicRpc.getInstanceAdminStubSettings().createStub()); + settings.createStub()); } catch (IOException ex) { throw SpannerExceptionFactory.newSpannerException(ex); } @@ -364,8 +374,10 @@ void setNextPageToken(String nextPageToken) { private com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient createDatabaseAdminClient() { try { + final DatabaseAdminStubSettings settings = + Preconditions.checkNotNull(gapicRpc.getDatabaseAdminStubSettings()); return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( - gapicRpc.getDatabaseAdminStubSettings().createStub()); + settings.createStub()); } catch (IOException ex) { throw SpannerExceptionFactory.newSpannerException(ex); } From 3dd70177ade41b93f3a186322e2787b3ced91ca5 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Tue, 13 Feb 2024 13:57:15 +0530 Subject: [PATCH 17/20] chore: change to create methods and avoid stale instances. --- .../clirr-ignored-differences.xml | 4 +- .../com/google/cloud/spanner/Spanner.java | 22 +++++--- .../com/google/cloud/spanner/SpannerImpl.java | 55 ++++++------------- .../it/ITAutogeneratedAdminClientTest.java | 4 +- 4 files changed, 37 insertions(+), 48 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index bae6d09eb1..fbbc0153f8 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -543,12 +543,12 @@ 7012 com/google/cloud/spanner/Spanner - com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() + com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient createDatabaseAdminClient() 7012 com/google/cloud/spanner/Spanner - com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() + com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient createInstanceAdminClient() 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 b29fbabf73..7ccbc88d97 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 @@ -45,7 +45,10 @@ public interface Spanner extends Service, AutoCloseable { /** * Returns a {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} to execute - * admin operations on Cloud Spanner databases. + * admin operations on Cloud Spanner databases. This method always creates a new instance of + * {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} which is an {@link + * AutoCloseable} resource. For optimising the number of clients, caller may choose to cache the + * clients instead of repeatedly invoking this method and creating new instances. * * @return {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} */ @@ -54,11 +57,12 @@ public interface Spanner extends Service, AutoCloseable { * 
{@code
    * SpannerOptions options = SpannerOptions.newBuilder().build();
    * Spanner spanner = options.getService();
-   * DatabaseAdminClient dbAdminClient = spanner.databaseAdminClient();
+   * DatabaseAdminClient dbAdminClient = spanner.createDatabaseAdminClient();
    * }
* */ - default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() { + default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient + createDatabaseAdminClient() { throw new UnsupportedOperationException("Not implemented"); } @@ -79,8 +83,11 @@ default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseA InstanceAdminClient getInstanceAdminClient(); /** - * Returns an {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} to execute - * admin operations on Cloud Spanner instances. + * Returns a {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} to execute + * admin operations on Cloud Spanner databases. This method always creates a new instance of + * {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} which is an {@link + * AutoCloseable} resource. For optimising the number of clients, caller may choose to cache the + * clients instead of repeatedly invoking this method and creating new instances. * * @return {@link com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient} */ @@ -89,11 +96,12 @@ default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseA * 
{@code
    * SpannerOptions options = SpannerOptions.newBuilder().build();
    * Spanner spanner = options.getService();
-   * InstanceAdminClient instanceAdminClient = spanner.instanceAdminClient();
+   * InstanceAdminClient instanceAdminClient = spanner.createInstanceAdminClient();
    * }
* */ - default com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() { + default com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient + createInstanceAdminClient() { throw new UnsupportedOperationException("Not implemented"); } 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 f47dc36890..2ab75d7417 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 @@ -110,9 +110,6 @@ private static String nextDatabaseClientId(DatabaseId databaseId) { private final DatabaseAdminClient dbAdminClient; private final InstanceAdminClient instanceClient; - private com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient; - private com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient; - /** * Exception class used to track the stack trace at the point when a Spanner instance is closed. * This exception will be thrown if a user tries to use any resources that were returned by this @@ -141,8 +138,6 @@ static final class ClosedException extends RuntimeException { this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), gapicRpc); this.instanceClient = new InstanceAdminClientImpl(options.getProjectId(), gapicRpc, dbAdminClient); - this.databaseAdminClient = createDatabaseAdminClient(); - this.instanceAdminClient = createInstanceAdminClient(); } SpannerImpl(SpannerOptions options) { @@ -216,11 +211,16 @@ public DatabaseAdminClient getDatabaseAdminClient() { } @Override - public com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient databaseAdminClient() { - if(databaseAdminClient.isTerminated() || databaseAdminClient.isShutdown()) { - this.databaseAdminClient = createDatabaseAdminClient(); + public com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient + createDatabaseAdminClient() { + try { + final DatabaseAdminStubSettings settings = + Preconditions.checkNotNull(gapicRpc.getDatabaseAdminStubSettings()); + return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( + settings.createStub()); + } catch (IOException ex) { + throw SpannerExceptionFactory.newSpannerException(ex); } - return this.databaseAdminClient; } @Override @@ -229,11 +229,16 @@ public InstanceAdminClient getInstanceAdminClient() { } @Override - public com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient instanceAdminClient() { - if(instanceAdminClient.isTerminated() || instanceAdminClient.isShutdown()) { - this.instanceAdminClient = createInstanceAdminClient(); + public com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient + createInstanceAdminClient() { + try { + final InstanceAdminStubSettings settings = + Preconditions.checkNotNull(gapicRpc.getInstanceAdminStubSettings()); + return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create( + settings.createStub()); + } catch (IOException ex) { + throw SpannerExceptionFactory.newSpannerException(ex); } - return this.instanceAdminClient; } @Override @@ -358,28 +363,4 @@ void setNextPageToken(String nextPageToken) { abstract S fromProto(T proto); } - - private com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient - createInstanceAdminClient() { - try { - final InstanceAdminStubSettings settings = - Preconditions.checkNotNull(gapicRpc.getInstanceAdminStubSettings()); - return com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient.create( - settings.createStub()); - } catch (IOException ex) { - throw SpannerExceptionFactory.newSpannerException(ex); - } - } - - private com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient - createDatabaseAdminClient() { - try { - final DatabaseAdminStubSettings settings = - Preconditions.checkNotNull(gapicRpc.getDatabaseAdminStubSettings()); - return com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient.create( - settings.createStub()); - } catch (IOException ex) { - throw SpannerExceptionFactory.newSpannerException(ex); - } - } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java index 374e8e440e..a1c6a35819 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAutogeneratedAdminClientTest.java @@ -86,8 +86,8 @@ public static List data() { public static void setUp() { assumeFalse("Emulator does not support database roles", isUsingEmulator()); testHelper = env.getTestHelper(); - dbAdminClient = testHelper.getClient().databaseAdminClient(); - instanceAdminClient = testHelper.getClient().instanceAdminClient(); + dbAdminClient = testHelper.getClient().createDatabaseAdminClient(); + instanceAdminClient = testHelper.getClient().createInstanceAdminClient(); databasesToDrop = new ArrayList<>(); } From 543fed9d4ec8bc0ee629e576285622205c1be021 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Tue, 13 Feb 2024 14:16:18 +0530 Subject: [PATCH 18/20] Revert "chore: fix broken unit tests." This reverts commit 07c9cd1121f2cf1f6e51d89a8981944325b2ad7b. --- .../cloud/spanner/BatchClientImplTest.java | 15 +----------- .../google/cloud/spanner/SessionImplTest.java | 15 +----------- .../google/cloud/spanner/SpannerImplTest.java | 15 +----------- .../spanner/TransactionManagerImplTest.java | 23 ++----------------- .../spanner/TransactionRunnerImplTest.java | 17 +------------- 5 files changed, 6 insertions(+), 79 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java index 75eb899aaa..b7c4834044 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java @@ -30,17 +30,12 @@ import com.google.cloud.Timestamp; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.protobuf.ByteString; import com.google.protobuf.util.Timestamps; import com.google.spanner.v1.Session; import com.google.spanner.v1.Transaction; import io.opentelemetry.api.OpenTelemetry; -import java.io.IOException; import java.util.Collections; import java.util.Map; import org.junit.Before; @@ -64,10 +59,6 @@ public final class BatchClientImplTest { @Mock private SpannerRpc gapicRpc; @Mock private SpannerOptions spannerOptions; - @Mock private InstanceAdminStubSettings instanceAdminStubSettings; - @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; - @Mock private DatabaseAdminStub databaseAdminStub; - @Mock private InstanceAdminStub instanceAdminStub; @Captor private ArgumentCaptor> optionsCaptor; @Mock private BatchTransactionId txnID; @@ -81,7 +72,7 @@ public static void setupOpenTelemetry() { @SuppressWarnings("unchecked") @Before - public void setUp() throws IOException { + public void setUp() { initMocks(this); DatabaseId db = DatabaseId.of(DB_NAME); when(spannerOptions.getNumChannels()).thenReturn(4); @@ -95,10 +86,6 @@ public void setUp() throws IOException { GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); when(transportOptions.getExecutorFactory()).thenReturn(mock(ExecutorFactory.class)); when(spannerOptions.getTransportOptions()).thenReturn(transportOptions); - when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); - when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); - when(gapicRpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); - when(gapicRpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); @SuppressWarnings("resource") SpannerImpl spanner = new SpannerImpl(gapicRpc, spannerOptions); client = new BatchClientImpl(spanner.getSessionClient(db)); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java index 59aa063d99..87edb64134 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java @@ -34,10 +34,6 @@ import com.google.cloud.Timestamp; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.protobuf.ByteString; @@ -56,7 +52,6 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Scope; -import java.io.IOException; import java.text.ParseException; import java.util.Calendar; import java.util.Collections; @@ -81,10 +76,6 @@ public class SessionImplTest { @Mock private SpannerRpc rpc; @Mock private SpannerOptions spannerOptions; - @Mock private InstanceAdminStubSettings instanceAdminStubSettings; - @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; - @Mock private DatabaseAdminStub databaseAdminStub; - @Mock private InstanceAdminStub instanceAdminStub; private com.google.cloud.spanner.Session session; @Captor private ArgumentCaptor> optionsCaptor; private Map options; @@ -97,7 +88,7 @@ public static void setupOpenTelemetry() { @SuppressWarnings("unchecked") @Before - public void setUp() throws IOException { + public void setUp() { MockitoAnnotations.initMocks(this); when(spannerOptions.getNumChannels()).thenReturn(4); when(spannerOptions.getPrefetchChunks()).thenReturn(1); @@ -110,10 +101,6 @@ public void setUp() throws IOException { when(spannerOptions.getTransportOptions()).thenReturn(transportOptions); when(spannerOptions.getSessionPoolOptions()).thenReturn(mock(SessionPoolOptions.class)); when(spannerOptions.getOpenTelemetry()).thenReturn(OpenTelemetry.noop()); - when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); - when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); - when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); - when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); @SuppressWarnings("resource") SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); String dbName = "projects/p1/instances/i1/databases/d1"; 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 dea2e3e320..31a6cad4c8 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 @@ -29,14 +29,9 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.SpannerException.DoNotConstructDirectly; import com.google.cloud.spanner.SpannerImpl.ClosedException; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; import io.opentelemetry.api.OpenTelemetry; -import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; import java.util.Collections; @@ -60,10 +55,6 @@ public class SpannerImplTest { @Mock private SpannerRpc rpc; @Mock private SpannerOptions spannerOptions; - @Mock private InstanceAdminStubSettings instanceAdminStubSettings; - @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; - @Mock private DatabaseAdminStub databaseAdminStub; - @Mock private InstanceAdminStub instanceAdminStub; private SpannerImpl impl; @Captor ArgumentCaptor> options; @@ -75,7 +66,7 @@ public static void setupOpenTelemetry() { } @Before - public void setUp() throws IOException { + public void setUp() { MockitoAnnotations.initMocks(this); when(spannerOptions.getNumChannels()).thenReturn(4); when(spannerOptions.getDatabaseRole()).thenReturn("role"); @@ -84,10 +75,6 @@ public void setUp() throws IOException { when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); when(spannerOptions.getSessionLabels()).thenReturn(Collections.emptyMap()); when(spannerOptions.getOpenTelemetry()).thenReturn(OpenTelemetry.noop()); - when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); - when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); - when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); - when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); impl = new SpannerImpl(rpc, spannerOptions); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java index fd6f57d484..dc28b333c4 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java @@ -34,10 +34,6 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; import com.google.cloud.spanner.TransactionManager.TransactionState; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.protobuf.ByteString; import com.google.protobuf.Empty; @@ -51,7 +47,6 @@ import com.google.spanner.v1.Session; import com.google.spanner.v1.Transaction; import io.opentelemetry.api.OpenTelemetry; -import java.io.IOException; import java.util.Collections; import java.util.UUID; import java.util.concurrent.Executors; @@ -82,10 +77,6 @@ public void release(ScheduledExecutorService exec) { @Mock private SessionImpl session; @Mock TransactionRunnerImpl.TransactionContextImpl txn; - @Mock private InstanceAdminStubSettings instanceAdminStubSettings; - @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; - @Mock private DatabaseAdminStub databaseAdminStub; - @Mock private InstanceAdminStub instanceAdminStub; private TransactionManagerImpl manager; @BeforeClass @@ -210,7 +201,7 @@ public void commitAfterRollbackFails() { @SuppressWarnings("unchecked") @Test - public void usesPreparedTransaction() throws IOException { + public void usesPreparedTransaction() { SpannerOptions options = mock(SpannerOptions.class); when(options.getNumChannels()).thenReturn(4); GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); @@ -257,11 +248,6 @@ public void usesPreparedTransaction() throws IOException { com.google.protobuf.Timestamp.newBuilder() .setSeconds(System.currentTimeMillis() * 1000)) .build())); - when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); - when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); - when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); - when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); - DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); @@ -277,7 +263,7 @@ public void usesPreparedTransaction() throws IOException { @SuppressWarnings({"unchecked", "resource"}) @Test - public void inlineBegin() throws IOException { + public void inlineBegin() { SpannerOptions options = mock(SpannerOptions.class); when(options.getNumChannels()).thenReturn(4); GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); @@ -346,11 +332,6 @@ public void inlineBegin() throws IOException { com.google.protobuf.Timestamp.newBuilder() .setSeconds(System.currentTimeMillis() * 1000)) .build())); - when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); - when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); - when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); - when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); - DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java index db92e4caa2..cf5f901f65 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java @@ -34,10 +34,6 @@ import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; import com.google.cloud.spanner.SessionClient.SessionId; import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.common.base.Preconditions; import com.google.protobuf.ByteString; @@ -67,7 +63,6 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Scope; -import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.UUID; @@ -86,7 +81,6 @@ /** Unit test for {@link com.google.cloud.spanner.TransactionRunnerImpl} */ @RunWith(JUnit4.class) public class TransactionRunnerImplTest { - private static final class TestExecutorFactory implements ExecutorFactory { @Override @@ -103,10 +97,6 @@ public void release(ScheduledExecutorService exec) { @Mock private SpannerRpc rpc; @Mock private SessionImpl session; @Mock private TransactionRunnerImpl.TransactionContextImpl txn; - @Mock private InstanceAdminStubSettings instanceAdminStubSettings; - @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; - @Mock private DatabaseAdminStub databaseAdminStub; - @Mock private InstanceAdminStub instanceAdminStub; private TransactionRunnerImpl transactionRunner; private boolean firstRun; private boolean usedInlinedBegin; @@ -161,7 +151,7 @@ public void setUp() { @SuppressWarnings("unchecked") @Test - public void usesPreparedTransaction() throws IOException { + public void usesPreparedTransaction() { SpannerOptions options = mock(SpannerOptions.class); when(options.getNumChannels()).thenReturn(4); GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); @@ -206,11 +196,6 @@ public void usesPreparedTransaction() throws IOException { .setCommitTimestamp( Timestamp.newBuilder().setSeconds(System.currentTimeMillis() * 1000)) .build())); - when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); - when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); - when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); - when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); - DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); From 53d4c7b10ecdb8ac4a11092c8c2dd58ce9aa9bc2 Mon Sep 17 00:00:00 2001 From: Arpan Mishra Date: Tue, 13 Feb 2024 14:45:07 +0530 Subject: [PATCH 19/20] test: add more unit tests. --- .../google/cloud/spanner/SpannerImplTest.java | 72 +++++++++++++++++++ .../spanner/spi/v1/GapicSpannerRpcTest.java | 31 ++++++++ 2 files changed, 103 insertions(+) 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 31a6cad4c8..3cf13dc58d 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 @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.when; @@ -29,9 +30,16 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.SpannerException.DoNotConstructDirectly; import com.google.cloud.spanner.SpannerImpl.ClosedException; +import com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; import io.opentelemetry.api.OpenTelemetry; +import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; import java.util.Collections; @@ -55,6 +63,10 @@ public class SpannerImplTest { @Mock private SpannerRpc rpc; @Mock private SpannerOptions spannerOptions; + @Mock private DatabaseAdminStubSettings databaseAdminStubSettings; + @Mock private DatabaseAdminStub databaseAdminStub; + @Mock private InstanceAdminStubSettings instanceAdminStubSettings; + @Mock private InstanceAdminStub instanceAdminStub; private SpannerImpl impl; @Captor ArgumentCaptor> options; @@ -286,6 +298,66 @@ public void testClosedException() { assertThat(sw.toString()).contains("closeSpannerAndIncludeStacktrace"); } + @Test + public void testCreateDatabaseAdminClient_whenNullAdminSettings_assertPreconditionFailure() { + Spanner spanner = new SpannerImpl(rpc, spannerOptions); + assertThrows(NullPointerException.class, spanner::createDatabaseAdminClient); + } + + @Test + public void testCreateDatabaseAdminClient_whenMockAdminSettings_assertMethodInvocation() + throws IOException { + when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); + when(databaseAdminStubSettings.createStub()).thenReturn(databaseAdminStub); + + Spanner spanner = new SpannerImpl(rpc, spannerOptions); + + DatabaseAdminClient databaseAdminClient = spanner.createDatabaseAdminClient(); + assertNotNull(databaseAdminClient); + } + + @Test(expected = SpannerException.class) + public void testCreateDatabaseAdminClient_whenMockAdminSettings_assertException() + throws IOException { + when(rpc.getDatabaseAdminStubSettings()).thenReturn(databaseAdminStubSettings); + when(databaseAdminStubSettings.createStub()).thenThrow(IOException.class); + + Spanner spanner = new SpannerImpl(rpc, spannerOptions); + + DatabaseAdminClient databaseAdminClient = spanner.createDatabaseAdminClient(); + assertNotNull(databaseAdminClient); + } + + @Test + public void testCreateInstanceAdminClient_whenNullAdminSettings_assertPreconditionFailure() { + Spanner spanner = new SpannerImpl(rpc, spannerOptions); + assertThrows(NullPointerException.class, spanner::createInstanceAdminClient); + } + + @Test + public void testCreateInstanceAdminClient_whenMockAdminSettings_assertMethodInvocation() + throws IOException { + when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(instanceAdminStubSettings.createStub()).thenReturn(instanceAdminStub); + + Spanner spanner = new SpannerImpl(rpc, spannerOptions); + + InstanceAdminClient instanceAdminClient = spanner.createInstanceAdminClient(); + assertNotNull(instanceAdminClient); + } + + @Test(expected = SpannerException.class) + public void testCreateInstanceAdminClient_whenMockAdminSettings_assertException() + throws IOException { + when(rpc.getInstanceAdminStubSettings()).thenReturn(instanceAdminStubSettings); + when(instanceAdminStubSettings.createStub()).thenThrow(IOException.class); + + Spanner spanner = new SpannerImpl(rpc, spannerOptions); + + InstanceAdminClient instanceAdminClient = spanner.createInstanceAdminClient(); + assertNotNull(instanceAdminClient); + } + private void closeSpannerAndIncludeStacktrace(Spanner spanner) { spanner.close(); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java index bbe23aff63..fb139dc89d 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java @@ -637,6 +637,37 @@ public void testCustomClientLibToken_alsoContainsDefaultToken() { Objects.requireNonNull(lastSeenHeaders.get(key)).contains("gl-java/")); } + @Test + public void testGetDatabaseAdminStubSettings_whenStubInitialized_assertNonNullClientSetting() { + SpannerOptions options = createSpannerOptions(); + GapicSpannerRpc rpc = new GapicSpannerRpc(options, true); + + assertNotNull(rpc.getDatabaseAdminStubSettings()); + + rpc.shutdown(); + } + + @Test + public void testGetInstanceAdminStubSettings_whenStubInitialized_assertNonNullClientSetting() { + SpannerOptions options = createSpannerOptions(); + GapicSpannerRpc rpc = new GapicSpannerRpc(options, true); + + assertNotNull(rpc.getInstanceAdminStubSettings()); + + rpc.shutdown(); + } + + @Test + public void testAdminStubSettings_whenStubNotInitialized_assertNullClientSetting() { + SpannerOptions options = createSpannerOptions(); + GapicSpannerRpc rpc = new GapicSpannerRpc(options, false); + + assertNull(rpc.getDatabaseAdminStubSettings()); + assertNull(rpc.getInstanceAdminStubSettings()); + + rpc.shutdown(); + } + private SpannerOptions createSpannerOptions() { String endpoint = address.getHostString() + ":" + server.getPort(); return SpannerOptions.newBuilder() From d8d6a5b63476a71aaf224eb7e9b6e9055243de24 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 13 Feb 2024 10:11:24 +0000 Subject: [PATCH 20/20] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 56d668b0a6..e70ee5eb3a 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ 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.31.0') +implementation platform('com.google.cloud:libraries-bom:26.32.0') implementation 'com.google.cloud:google-cloud-spanner' ```