Skip to content

Commit 02b88a1

Browse files
shobhitsgshobhitsg
committed
feat: addressed review comments and added missing javadoc
1 parent 722feb3 commit 02b88a1

File tree

8 files changed

+99
-75
lines changed

8 files changed

+99
-75
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public static TransactionOption optimisticLock() {
163163
/**
164164
* Specifying this instructs the transaction to request {@link IsolationLevel} from the backend.
165165
*/
166-
public static IsolationLevelOption isolationLevelOption(IsolationLevel isolationLevel) {
166+
public static TransactionOption isolationLevelOption(IsolationLevel isolationLevel) {
167167
return new IsolationLevelOption(isolationLevel);
168168
}
169169

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.google.cloud.spanner.Options.TransactionOption;
3131
import com.google.cloud.spanner.Options.UpdateOption;
3232
import com.google.cloud.spanner.SessionClient.SessionOption;
33-
import com.google.cloud.spanner.SpannerOptions.Builder.TransactionOptions;
3433
import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl;
3534
import com.google.cloud.spanner.spi.v1.SpannerRpc;
3635
import com.google.common.base.Ticker;
@@ -45,6 +44,7 @@
4544
import com.google.spanner.v1.CommitRequest;
4645
import com.google.spanner.v1.RequestOptions;
4746
import com.google.spanner.v1.Transaction;
47+
import com.google.spanner.v1.TransactionOptions;
4848
import java.time.Instant;
4949
import java.util.ArrayList;
5050
import java.util.Collection;
@@ -69,18 +69,15 @@ static void throwIfTransactionsPending() {
6969
}
7070
}
7171

72-
static com.google.spanner.v1.TransactionOptions createReadWriteTransactionOptions(
72+
static TransactionOptions createReadWriteTransactionOptions(
7373
Options options, ByteString previousTransactionId) {
74-
com.google.spanner.v1.TransactionOptions.Builder transactionOptions =
75-
com.google.spanner.v1.TransactionOptions.newBuilder();
74+
TransactionOptions.Builder transactionOptions = TransactionOptions.newBuilder();
7675
if (options.withExcludeTxnFromChangeStreams() == Boolean.TRUE) {
7776
transactionOptions.setExcludeTxnFromChangeStreams(true);
7877
}
79-
com.google.spanner.v1.TransactionOptions.ReadWrite.Builder readWrite =
80-
com.google.spanner.v1.TransactionOptions.ReadWrite.newBuilder();
78+
TransactionOptions.ReadWrite.Builder readWrite = TransactionOptions.ReadWrite.newBuilder();
8179
if (options.withOptimisticLock() == Boolean.TRUE) {
82-
readWrite.setReadLockMode(
83-
com.google.spanner.v1.TransactionOptions.ReadWrite.ReadLockMode.OPTIMISTIC);
80+
readWrite.setReadLockMode(TransactionOptions.ReadWrite.ReadLockMode.OPTIMISTIC);
8481
}
8582
if (previousTransactionId != null
8683
&& previousTransactionId != com.google.protobuf.ByteString.EMPTY) {
@@ -199,12 +196,8 @@ void markUsed(Instant instant) {
199196
sessionReference.markUsed(instant);
200197
}
201198

202-
com.google.spanner.v1.TransactionOptions defaultTransactionOptions() {
203-
TransactionOptions transactionOptions =
204-
this.spanner.getOptions().getDefaultTransactionOptions();
205-
return transactionOptions != null
206-
? transactionOptions.getTransactionOptions()
207-
: com.google.spanner.v1.TransactionOptions.getDefaultInstance();
199+
TransactionOptions defaultTransactionOptions() {
200+
return this.spanner.getOptions().getDefaultTransactionOptions();
208201
}
209202

210203
public DatabaseId getDatabaseId() {
@@ -260,17 +253,17 @@ public CommitResponse writeAtLeastOnceWithOptions(
260253
.setReturnCommitStats(options.withCommitStats())
261254
.addAllMutations(mutationsProto);
262255

263-
com.google.spanner.v1.TransactionOptions.Builder transactionOptionsBuilder =
264-
com.google.spanner.v1.TransactionOptions.newBuilder()
265-
.setReadWrite(com.google.spanner.v1.TransactionOptions.ReadWrite.getDefaultInstance());
256+
TransactionOptions.Builder transactionOptionsBuilder =
257+
TransactionOptions.newBuilder()
258+
.setReadWrite(TransactionOptions.ReadWrite.getDefaultInstance());
266259
if (options.withExcludeTxnFromChangeStreams() == Boolean.TRUE) {
267260
transactionOptionsBuilder.setExcludeTxnFromChangeStreams(true);
268261
}
269262
if (options.isolationLevel() != null) {
270263
transactionOptionsBuilder.setIsolationLevel(options.isolationLevel());
271264
}
272265
requestBuilder.setSingleUseTransaction(
273-
this.defaultTransactionOptions().toBuilder().mergeFrom(transactionOptionsBuilder.build()));
266+
defaultTransactionOptions().toBuilder().mergeFrom(transactionOptionsBuilder.build()));
274267

275268
if (options.hasMaxCommitDelay()) {
276269
requestBuilder.setMaxCommitDelay(

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java‎

Lines changed: 44 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,8 @@
4444
import com.google.cloud.grpc.GcpManagedChannelOptions;
4545
import com.google.cloud.grpc.GrpcTransportOptions;
4646
import com.google.cloud.spanner.Options.DirectedReadOption;
47-
import com.google.cloud.spanner.Options.IsolationLevelOption;
4847
import com.google.cloud.spanner.Options.QueryOption;
4948
import com.google.cloud.spanner.Options.UpdateOption;
50-
import com.google.cloud.spanner.SpannerOptions.Builder.TransactionOptions;
5149
import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings;
5250
import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings;
5351
import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings;
@@ -68,6 +66,8 @@
6866
import com.google.spanner.v1.ExecuteSqlRequest;
6967
import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions;
7068
import com.google.spanner.v1.SpannerGrpc;
69+
import com.google.spanner.v1.TransactionOptions;
70+
import com.google.spanner.v1.TransactionOptions.IsolationLevel;
7171
import io.grpc.CallCredentials;
7272
import io.grpc.CompressorRegistry;
7373
import io.grpc.Context;
@@ -180,7 +180,7 @@ public class SpannerOptions extends ServiceOptions<Spanner, SpannerOptions> {
180180
private final boolean enableExtendedTracing;
181181
private final boolean enableEndToEndTracing;
182182
private final String monitoringHost;
183-
private final TransactionOptions transactionOptions;
183+
private final TransactionOptions defaultTransactionOptions;
184184

185185
enum TracingFramework {
186186
OPEN_CENSUS,
@@ -810,7 +810,11 @@ protected SpannerOptions(Builder builder) {
810810
enableBuiltInMetrics = builder.enableBuiltInMetrics;
811811
enableEndToEndTracing = builder.enableEndToEndTracing;
812812
monitoringHost = builder.monitoringHost;
813-
transactionOptions = builder.transactionOptions;
813+
if (builder.defaultTransactionOptions == null) {
814+
defaultTransactionOptions = TransactionOptions.getDefaultInstance();
815+
} else {
816+
defaultTransactionOptions = builder.defaultTransactionOptions;
817+
}
814818
}
815819

816820
/**
@@ -992,7 +996,7 @@ public static class Builder
992996
private String monitoringHost = SpannerOptions.environment.getMonitoringHost();
993997
private SslContext mTLSContext = null;
994998
private boolean isExperimentalHost = false;
995-
private TransactionOptions transactionOptions;
999+
private TransactionOptions defaultTransactionOptions;
9961000

9971001
private static String createCustomClientLibToken(String token) {
9981002
return token + " " + ServiceOptions.getGoogApiClientLibName();
@@ -1061,7 +1065,7 @@ protected Builder() {
10611065
this.enableBuiltInMetrics = options.enableBuiltInMetrics;
10621066
this.enableEndToEndTracing = options.enableEndToEndTracing;
10631067
this.monitoringHost = options.monitoringHost;
1064-
this.transactionOptions = options.transactionOptions;
1068+
this.defaultTransactionOptions = options.defaultTransactionOptions;
10651069
}
10661070

10671071
@Override
@@ -1651,42 +1655,51 @@ public Builder setEnableEndToEndTracing(boolean enableEndToEndTracing) {
16511655
return this;
16521656
}
16531657

1654-
public static class TransactionOptions {
1655-
private com.google.spanner.v1.TransactionOptions transactionOptions;
1656-
1657-
private TransactionOptions() {}
1658+
/**
1659+
* Provides the default read-write transaction options for all databases, limited to setting the
1660+
* {@link IsolationLevel}. These defaults are overridden by any explicit {@link
1661+
* com.google.cloud.spanner.Options.TransactionOption} provided through {@link DatabaseClient}.
1662+
*
1663+
* <p>Example Usage:
1664+
*
1665+
* <pre>{@code
1666+
* DefaultReadWriteTransactionOptions options = DefaultReadWriteTransactionOptions.newBuilder()
1667+
* .setIsolationLevel(IsolationLevel.SERIALIZABLE)
1668+
* .build();
1669+
* }</pre>
1670+
*/
1671+
public static class DefaultReadWriteTransactionOptions {
1672+
private final TransactionOptions defaultTransactionOptions;
16581673

1659-
com.google.spanner.v1.TransactionOptions getTransactionOptions() {
1660-
return transactionOptions;
1674+
private DefaultReadWriteTransactionOptions(TransactionOptions defaultTransactionOptions) {
1675+
this.defaultTransactionOptions = defaultTransactionOptions;
16611676
}
16621677

1663-
public static class TransactionOptionsBuilder {
1664-
private IsolationLevelOption isolationLevelOption;
1678+
public static DefaultReadWriteTransactionOptionsBuilder newBuilder() {
1679+
return new DefaultReadWriteTransactionOptionsBuilder();
1680+
}
16651681

1666-
public static TransactionOptionsBuilder newBuilder() {
1667-
return new TransactionOptionsBuilder();
1668-
}
1682+
public static class DefaultReadWriteTransactionOptionsBuilder {
1683+
private final TransactionOptions.Builder transactionOptionsBuilder =
1684+
TransactionOptions.newBuilder();
16691685

1670-
public TransactionOptionsBuilder setIsolationLevel(IsolationLevelOption option) {
1671-
this.isolationLevelOption = option;
1686+
public DefaultReadWriteTransactionOptionsBuilder setIsolationLevel(
1687+
IsolationLevel isolationLevel) {
1688+
transactionOptionsBuilder.setIsolationLevel(isolationLevel);
16721689
return this;
16731690
}
16741691

1675-
public TransactionOptions build() {
1676-
TransactionOptions transactionOptions = new TransactionOptions();
1677-
transactionOptions.transactionOptions =
1678-
com.google.spanner.v1.TransactionOptions.newBuilder()
1679-
.setIsolationLevel(
1680-
Options.fromTransactionOptions(isolationLevelOption).isolationLevel())
1681-
.build();
1682-
return transactionOptions;
1692+
public DefaultReadWriteTransactionOptions build() {
1693+
return new DefaultReadWriteTransactionOptions(transactionOptionsBuilder.build());
16831694
}
16841695
}
16851696
}
16861697

1687-
/** Sets the default transaction options. */
1688-
public Builder setDefaultTransactionOptions(TransactionOptions transactionOptions) {
1689-
this.transactionOptions = transactionOptions;
1698+
/** Sets the {@link DefaultReadWriteTransactionOptions} for read-write transactions. */
1699+
public Builder setDefaultTransactionOptions(
1700+
DefaultReadWriteTransactionOptions defaultReadWriteTransactionOptions) {
1701+
Preconditions.checkNotNull(defaultReadWriteTransactionOptions);
1702+
this.defaultTransactionOptions = defaultReadWriteTransactionOptions.defaultTransactionOptions;
16901703
return this;
16911704
}
16921705

@@ -2036,7 +2049,7 @@ String getMonitoringHost() {
20362049
}
20372050

20382051
public TransactionOptions getDefaultTransactionOptions() {
2039-
return transactionOptions;
2052+
return defaultTransactionOptions;
20402053
}
20412054

20422055
@BetaApi

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplWithTransactionOptionsTest.java‎ renamed to ‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplWithDefaultRWTransactionOptionsTest.java‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727
import com.google.api.gax.grpc.testing.LocalChannelProvider;
2828
import com.google.cloud.NoCredentials;
2929
import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
30-
import com.google.cloud.spanner.Options.IsolationLevelOption;
3130
import com.google.cloud.spanner.Options.RpcPriority;
32-
import com.google.cloud.spanner.SpannerOptions.Builder.TransactionOptions.TransactionOptionsBuilder;
31+
import com.google.cloud.spanner.Options.TransactionOption;
32+
import com.google.cloud.spanner.SpannerOptions.Builder.DefaultReadWriteTransactionOptions;
3333
import com.google.protobuf.AbstractMessage;
3434
import com.google.spanner.v1.BeginTransactionRequest;
3535
import com.google.spanner.v1.CommitRequest;
@@ -55,16 +55,20 @@
5555
import org.junit.runners.JUnit4;
5656

5757
@RunWith(JUnit4.class)
58-
public class DatabaseClientImplWithTransactionOptionsTest {
59-
private static final IsolationLevelOption SERIALIZABLE_ISOLATION_OPTION =
58+
public class DatabaseClientImplWithDefaultRWTransactionOptionsTest {
59+
private static final TransactionOption SERIALIZABLE_ISOLATION_OPTION =
6060
Options.isolationLevelOption(IsolationLevel.SERIALIZABLE);
61-
private static final IsolationLevelOption RR_ISOLATION_OPTION =
61+
private static final TransactionOption RR_ISOLATION_OPTION =
6262
Options.isolationLevelOption(IsolationLevel.REPEATABLE_READ);
63+
private static final DatabaseId DATABASE_ID =
64+
DatabaseId.of("[PROJECT]", "[INSTANCE]", "[DATABASE]");
6365
private static MockSpannerServiceImpl mockSpanner;
6466
private static Server server;
6567
private static ExecutorService executor;
6668
private static LocalChannelProvider channelProvider;
6769
private Spanner spanner;
70+
private Spanner spannerWithRR;
71+
private Spanner spannerWithSerializable;
6872
private DatabaseClient client;
6973
private DatabaseClient clientWithRepeatableReadOption;
7074
private DatabaseClient clientWithSerializableOption;
@@ -111,25 +115,25 @@ public void setUp() {
111115
.setChannelProvider(channelProvider)
112116
.setCredentials(NoCredentials.getInstance());
113117
spanner = spannerOptionsBuilder.build().getService();
114-
client = spanner.getDatabaseClient(DatabaseId.of("[PROJECT]", "[INSTANCE]", "[DATABASE]"));
115-
clientWithRepeatableReadOption =
118+
spannerWithRR =
116119
spannerOptionsBuilder
117120
.setDefaultTransactionOptions(
118-
TransactionOptionsBuilder.newBuilder()
119-
.setIsolationLevel(RR_ISOLATION_OPTION)
121+
DefaultReadWriteTransactionOptions.newBuilder()
122+
.setIsolationLevel(IsolationLevel.REPEATABLE_READ)
120123
.build())
121124
.build()
122-
.getService()
123-
.getDatabaseClient(DatabaseId.of("[PROJECT]", "[INSTANCE]", "[DATABASE]"));
124-
clientWithSerializableOption =
125+
.getService();
126+
spannerWithSerializable =
125127
spannerOptionsBuilder
126128
.setDefaultTransactionOptions(
127-
TransactionOptionsBuilder.newBuilder()
128-
.setIsolationLevel(SERIALIZABLE_ISOLATION_OPTION)
129+
DefaultReadWriteTransactionOptions.newBuilder()
130+
.setIsolationLevel(IsolationLevel.SERIALIZABLE)
129131
.build())
130132
.build()
131-
.getService()
132-
.getDatabaseClient(DatabaseId.of("[PROJECT]", "[INSTANCE]", "[DATABASE]"));
133+
.getService();
134+
client = spanner.getDatabaseClient(DATABASE_ID);
135+
clientWithRepeatableReadOption = spannerWithRR.getDatabaseClient(DATABASE_ID);
136+
clientWithSerializableOption = spannerWithSerializable.getDatabaseClient(DATABASE_ID);
133137
}
134138

135139
private void executeTest(
@@ -153,6 +157,8 @@ private void executeTestWithSerializable(
153157
@After
154158
public void tearDown() {
155159
spanner.close();
160+
spannerWithRR.close();
161+
spannerWithSerializable.close();
156162
}
157163

158164
@Test

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.google.spanner.v1.RollbackRequest;
4949
import com.google.spanner.v1.Session;
5050
import com.google.spanner.v1.Transaction;
51+
import com.google.spanner.v1.TransactionOptions;
5152
import io.opentelemetry.api.OpenTelemetry;
5253
import io.opentelemetry.api.trace.Span;
5354
import io.opentelemetry.context.Scope;
@@ -90,6 +91,8 @@ public static void setupOpenTelemetry() {
9091
public void setUp() {
9192
MockitoAnnotations.initMocks(this);
9293
when(spannerOptions.getNumChannels()).thenReturn(4);
94+
when(spannerOptions.getDefaultTransactionOptions())
95+
.thenReturn(TransactionOptions.getDefaultInstance());
9396
when(spannerOptions.getPrefetchChunks()).thenReturn(1);
9497
when(spannerOptions.getDatabaseRole()).thenReturn("role");
9598
when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build());

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@
3737
import com.google.cloud.NoCredentials;
3838
import com.google.cloud.ServiceOptions;
3939
import com.google.cloud.TransportOptions;
40-
import com.google.cloud.spanner.SpannerOptions.Builder.TransactionOptions;
41-
import com.google.cloud.spanner.SpannerOptions.Builder.TransactionOptions.TransactionOptionsBuilder;
40+
import com.google.cloud.spanner.SpannerOptions.Builder.DefaultReadWriteTransactionOptions;
4241
import com.google.cloud.spanner.SpannerOptions.FixedCloseableExecutorProvider;
4342
import com.google.cloud.spanner.SpannerOptions.SpannerCallContextTimeoutConfigurator;
4443
import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings;
@@ -772,19 +771,20 @@ public void testMonitoringHost() {
772771

773772
@Test
774773
public void testTransactionOptions() {
775-
TransactionOptions transactionOptions =
776-
TransactionOptionsBuilder.newBuilder()
777-
.setIsolationLevel(Options.isolationLevelOption(IsolationLevel.SERIALIZABLE))
774+
DefaultReadWriteTransactionOptions transactionOptions =
775+
DefaultReadWriteTransactionOptions.newBuilder()
776+
.setIsolationLevel(IsolationLevel.SERIALIZABLE)
778777
.build();
779-
assertNull(
778+
assertNotNull(
780779
SpannerOptions.newBuilder().setProjectId("p").build().getDefaultTransactionOptions());
781780
assertThat(
782781
SpannerOptions.newBuilder()
783782
.setProjectId("p")
784783
.setDefaultTransactionOptions(transactionOptions)
785784
.build()
786-
.getDefaultTransactionOptions())
787-
.isEqualTo(transactionOptions);
785+
.getDefaultTransactionOptions()
786+
.getIsolationLevel())
787+
.isEqualTo(IsolationLevel.SERIALIZABLE);
788788
}
789789

790790
@Test

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import com.google.spanner.v1.ResultSetStats;
5050
import com.google.spanner.v1.Session;
5151
import com.google.spanner.v1.Transaction;
52+
import com.google.spanner.v1.TransactionOptions;
5253
import io.opentelemetry.api.OpenTelemetry;
5354
import java.util.Collections;
5455
import java.util.UUID;
@@ -207,6 +208,8 @@ public void commitAfterRollbackFails() {
207208
public void usesPreparedTransaction() {
208209
SpannerOptions options = mock(SpannerOptions.class);
209210
when(options.getNumChannels()).thenReturn(4);
211+
when(options.getDefaultTransactionOptions())
212+
.thenReturn(TransactionOptions.getDefaultInstance());
210213
GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class);
211214
when(transportOptions.getExecutorFactory()).thenReturn(new TestExecutorFactory());
212215
when(options.getTransportOptions()).thenReturn(transportOptions);
@@ -288,6 +291,8 @@ public void inlineBegin() {
288291
when(options.getNumChannels()).thenReturn(4);
289292
GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class);
290293
when(transportOptions.getExecutorFactory()).thenReturn(new TestExecutorFactory());
294+
when(options.getDefaultTransactionOptions())
295+
.thenReturn(TransactionOptions.getDefaultInstance());
291296
when(options.getTransportOptions()).thenReturn(transportOptions);
292297
SessionPoolOptions sessionPoolOptions =
293298
SessionPoolOptions.newBuilder().setMinSessions(0).setIncStep(1).build();

0 commit comments

Comments
 (0)