Skip to content

Commit

Permalink
feat: support public methods to use autogenerated admin clients. (#2878)
Browse files Browse the repository at this point in the history
* fix: prevent illegal negative timeout values into thread sleep() method while retrying exceptions in unit tests.

* For details on issue see - #2206

* Fixing lint issues.

* feat: support emulator with autogenerated admin clients.

* chore: modifying public interfaces and adding an integration test.

* chore: add deprecated annotations and add docs.

* chore: making interface methods default.

* chore: add clirr ignore checks for public methods.

* Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>

* Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>

* Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>

* Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Spanner.java

Co-authored-by: Knut Olav Løite <koloite@gmail.com>

* chore: address PR comments.

* chore: decouple client stub objects and rely on common stub settings object.

* chore: remove unused map objects.

* chore: fix broken unit tests.

* chore: handle case when client is terminated/shutdown

* chore: change to create methods and avoid stale instances.

* Revert "chore: fix broken unit tests."

This reverts commit 07c9cd1.

* test: add more unit tests.

* 🦉 Updates from OwlBot post-processor

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

---------

Co-authored-by: Knut Olav Løite <koloite@gmail.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Feb 13, 2024
1 parent 0e96d1f commit 53bcb3e
Show file tree
Hide file tree
Showing 9 changed files with 541 additions and 15 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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'
```
Expand Down
27 changes: 25 additions & 2 deletions google-cloud-spanner/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,30 @@
<className>com/google/cloud/spanner/Dialect</className>
<method>java.lang.String getDefaultSchema()</method>
</difference>
<!-- Exposing auto-generated admin/instance clients for admin operations -->
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/Spanner</className>
<method>com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient createDatabaseAdminClient()</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/Spanner</className>
<method>com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient createInstanceAdminClient()</method>
</difference>

<!-- (Internal change, use stub objects for generating auto-generated client object) -->
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/spi/v1/SpannerRpc</className>
<method>com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings getDatabaseAdminStubSettings()</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/spi/v1/SpannerRpc</className>
<method>com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings getInstanceAdminStubSettings()</method>
</difference>

<difference>
<differenceType>7005</differenceType>
<className>com/google/cloud/spanner/PartitionedDmlTransaction</className>
Expand All @@ -556,6 +580,5 @@
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/connection/Connection</className>
<method>void setDirectedRead(com.google.spanner.v1.DirectedReadOptions)</method>
</difference>

</difference>
</differences>
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@
* quota.
*/
public interface Spanner extends Service<SpannerOptions>, 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 {@code DatabaseAdminClient}
*/
/*
* <!--SNIPPET get_dbadmin_client-->
* <pre>{@code
Expand All @@ -38,7 +43,34 @@ public interface Spanner extends Service<SpannerOptions>, AutoCloseable {
*/
DatabaseAdminClient getDatabaseAdminClient();

/** Returns an {@code InstanceAdminClient} to do admin operations on Cloud Spanner instances. */
/**
* Returns a {@link com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient} to execute
* 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}
*/
/*
* <!--SNIPPET get_dbadmin_client-->
* <pre>{@code
* SpannerOptions options = SpannerOptions.newBuilder().build();
* Spanner spanner = options.getService();
* DatabaseAdminClient dbAdminClient = spanner.createDatabaseAdminClient();
* }</pre>
* <!--SNIPPET get_dbadmin_client-->
*/
default com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient
createDatabaseAdminClient() {
throw new UnsupportedOperationException("Not implemented");
}

/**
* Returns an {@code InstanceAdminClient} to execute admin operations on Cloud Spanner instances.
*
* @return {@code InstanceAdminClient}
*/
/*
* <!--SNIPPET get_instance_admin_client-->
* <pre>{@code
Expand All @@ -50,6 +82,29 @@ public interface Spanner extends Service<SpannerOptions>, AutoCloseable {
*/
InstanceAdminClient getInstanceAdminClient();

/**
* 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}
*/
/*
* <!--SNIPPET get_instance_admin_client-->
* <pre>{@code
* SpannerOptions options = SpannerOptions.newBuilder().build();
* Spanner spanner = options.getService();
* InstanceAdminClient instanceAdminClient = spanner.createInstanceAdminClient();
* }</pre>
* <!--SNIPPET get_instance_admin_client-->
*/
default com.google.cloud.spanner.admin.instance.v1.InstanceAdminClient
createInstanceAdminClient() {
throw new UnsupportedOperationException("Not implemented");
}

/**
* Returns a {@code DatabaseClient} for the given database. It uses a pool of sessions to talk to
* the database.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -40,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;
Expand Down Expand Up @@ -207,11 +210,37 @@ public DatabaseAdminClient getDatabaseAdminClient() {
return dbAdminClient;
}

@Override
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);
}
}

@Override
public InstanceAdminClient getInstanceAdminClient() {
return instanceClient;
}

@Override
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);
}
}

@Override
public DatabaseClient getDatabaseClient(DatabaseId db) {
synchronized (this) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ public class GapicSpannerRpc implements SpannerRpc {
private final Set<Code> 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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -510,6 +510,7 @@ public <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> createUnaryCalla
this.executeQueryRetryableCodes = null;
this.partitionedDmlStub = null;
this.databaseAdminStubSettings = null;
this.instanceAdminStubSettings = null;
this.spannerWatchdog = null;
this.partitionedDmlRetrySettings = null;
}
Expand Down Expand Up @@ -2004,6 +2005,16 @@ public boolean isClosed() {
return rpcIsClosed;
}

@Override
public DatabaseAdminStubSettings getDatabaseAdminStubSettings() {
return databaseAdminStubSettings;
}

@Override
public InstanceAdminStubSettings getInstanceAdminStubSettings() {
return instanceAdminStubSettings;
}

private static final class GrpcStreamingCall implements StreamingCall {
private final ApiCallContext callContext;
private final StreamController controller;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -500,4 +502,24 @@ TestIamPermissionsResponse testInstanceAdminIAMPermissions(
void shutdown();

boolean isClosed();

/**
* Getter method to obtain the auto-generated instance admin client stub settings.
*
* @return InstanceAdminStubSettings
*/
@InternalApi
default InstanceAdminStubSettings getInstanceAdminStubSettings() {
throw new UnsupportedOperationException("Not implemented");
}

/**
* Getter method to obtain the auto-generated database admin client stub settings.
*
* @return DatabaseAdminStubSettings
*/
@InternalApi
default DatabaseAdminStubSettings getDatabaseAdminStubSettings() {
throw new UnsupportedOperationException("Not implemented");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -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<Map<SpannerRpc.Option, Object>> options;
Expand Down Expand Up @@ -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();
}
Expand Down
Loading

0 comments on commit 53bcb3e

Please sign in to comment.