From f2844487561d0a64678d7e0a242ea8992f96343c Mon Sep 17 00:00:00 2001 From: rajatrb Date: Mon, 10 Apr 2023 10:46:40 +0000 Subject: [PATCH 01/18] feat: add support for Directed Read options --- .../cloud/spanner/AbstractReadContext.java | 12 ++++- .../com/google/cloud/spanner/Options.java | 32 ++++++++++- .../google/cloud/spanner/SpannerOptions.java | 17 ++++++ .../cloud/spanner/TransactionRunnerImpl.java | 4 +- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 53 +++++++++++++++++-- .../cloud/spanner/spi/v1/SpannerRpc.java | 15 ++++-- .../AsyncTransactionManagerImplTest.java | 1 - .../google/cloud/spanner/SessionImplTest.java | 3 +- .../google/cloud/spanner/SessionPoolTest.java | 7 ++- .../spanner/TransactionManagerImplTest.java | 7 ++- .../spanner/TransactionRunnerImplTest.java | 3 +- 11 files changed, 133 insertions(+), 21 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index 7facd19c826..3270a8f40b4 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -598,6 +598,10 @@ ExecuteSqlRequest.Builder getExecuteSqlRequestBuilder( if (options.hasDataBoostEnabled()) { builder.setDataBoostEnabled(options.dataBoostEnabled()); } + if (options.hasDirectedReadOptions()) { + SpannerUtil.verifyDirectedReadOptions(options.directedReadOptions()); + builder.setDirectedReadOptions(options.directedReadOptions()); + } builder.setSeqno(getSeqNo()); builder.setQueryOptions(buildQueryOptions(statement.getQueryOptions())); builder.setRequestOptions(buildRequestOptions(options)); @@ -667,7 +671,7 @@ CloseableIterator startStream(@Nullable ByteString resumeToken request.setTransaction(selector); } SpannerRpc.StreamingCall call = - rpc.executeQuery(request.build(), stream.consumer(), session.getOptions()); + rpc.executeQuery(request.build(), stream.consumer(), session.getOptions(), true); call.request(prefetchChunks); stream.setCall(call, request.getTransaction().hasBegin()); return stream; @@ -779,6 +783,10 @@ ResultSet readInternalWithOptions( if (readOptions.hasDataBoostEnabled()) { builder.setDataBoostEnabled(readOptions.dataBoostEnabled()); } + if (readOptions.hasDirectedReadOptions()) { + SpannerUtil.verifyDirectedReadOptions(readOptions.directedReadOptions()); + builder.setDirectedReadOptions(readOptions.directedReadOptions()); + } final int prefetchChunks = readOptions.hasPrefetchChunks() ? readOptions.prefetchChunks() : defaultPrefetchChunks; ResumableStreamIterator stream = @@ -798,7 +806,7 @@ CloseableIterator startStream(@Nullable ByteString resumeToken } builder.setRequestOptions(buildRequestOptions(readOptions)); SpannerRpc.StreamingCall call = - rpc.read(builder.build(), stream.consumer(), session.getOptions()); + rpc.read(builder.build(), stream.consumer(), session.getOptions(), true); call.request(prefetchChunks); stream.setCall(call, /* withBeginTransaction = */ builder.getTransaction().hasBegin()); return stream; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index 9712b508d5f..99ee2277bfa 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -18,6 +18,7 @@ import com.google.api.core.BetaApi; import com.google.common.base.Preconditions; +import com.google.spanner.v1.DirectedReadOptions; import com.google.spanner.v1.RequestOptions.Priority; import java.io.Serializable; import java.util.Objects; @@ -328,6 +329,19 @@ void appendToOptions(Options options) { } } + static final class DirectedReadOption extends InternalOption implements ReadAndQueryOption { + private final DirectedReadOptions directedReadOptions; + + DirectedReadOption(DirectedReadOptions directedReadOptions) { + this.directedReadOptions = directedReadOptions; + } + + @Override + void appendToOptions(Options options) { + options.directedReadOptions = directedReadOptions; + } + } + private boolean withCommitStats; private Long limit; private Integer prefetchChunks; @@ -341,6 +355,7 @@ void appendToOptions(Options options) { private Boolean validateOnly; private Boolean withOptimisticLock; private Boolean dataBoostEnabled; + private DirectedReadOptions directedReadOptions; // Construction is via factory methods below. private Options() {} @@ -441,6 +456,14 @@ Boolean dataBoostEnabled() { return dataBoostEnabled; } + boolean hasDirectedReadOptions() { + return directedReadOptions != null; + } + + DirectedReadOptions directedReadOptions() { + return directedReadOptions; + } + @Override public String toString() { StringBuilder b = new StringBuilder(); @@ -480,6 +503,9 @@ public String toString() { if (dataBoostEnabled != null) { b.append("dataBoostEnabled: ").append(dataBoostEnabled).append(' '); } + if (directedReadOptions != null) { + b.append("directedReadOptions: ").append(directedReadOptions).append(' '); + } return b.toString(); } @@ -515,7 +541,8 @@ public boolean equals(Object o) { && Objects.equals(etag(), that.etag()) && Objects.equals(validateOnly(), that.validateOnly()) && Objects.equals(withOptimisticLock(), that.withOptimisticLock()) - && Objects.equals(dataBoostEnabled(), that.dataBoostEnabled()); + && Objects.equals(dataBoostEnabled(), that.dataBoostEnabled()) + && Objects.equals(directedReadOptions(), that.directedReadOptions()); } @Override @@ -560,6 +587,9 @@ public int hashCode() { if (dataBoostEnabled != null) { result = 31 * result + dataBoostEnabled.hashCode(); } + if (directedReadOptions != null) { + result = 31 * result + directedReadOptions.hashCode(); + } return result; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index d6243f59592..d30cb8c25bc 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -47,6 +47,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.google.spanner.v1.DirectedReadOptions; import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; import com.google.spanner.v1.SpannerGrpc; @@ -132,6 +133,7 @@ public class SpannerOptions extends ServiceOptions { private final CallCredentialsProvider callCredentialsProvider; private final CloseableExecutorProvider asyncExecutorProvider; private final String compressorName; + private final DirectedReadOptions directedReadOptions; /** * Interface that can be used to provide {@link CallCredentials} instead of {@link Credentials} to @@ -600,6 +602,7 @@ private SpannerOptions(Builder builder) { callCredentialsProvider = builder.callCredentialsProvider; asyncExecutorProvider = builder.asyncExecutorProvider; compressorName = builder.compressorName; + directedReadOptions = builder.directedReadOptions; } /** @@ -700,6 +703,7 @@ public static class Builder private CloseableExecutorProvider asyncExecutorProvider; private String compressorName; private String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST"); + private DirectedReadOptions directedReadOptions; private Builder() { // Manually set retry and polling settings that work. @@ -1081,6 +1085,12 @@ public Builder setCompressorName(@Nullable String compressorName) { return this; } + public Builder setDirectedReadOptions(DirectedReadOptions directedReadOptions) { + Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); + this.directedReadOptions = directedReadOptions; + return this; + } + /** * Sets the {@link ExecutorProvider} to use for high-level async calls that need an executor, * such as fetching results for an {@link AsyncResultSet}. @@ -1174,6 +1184,9 @@ public SpannerOptions build() { this.numChannels = this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS; } + if (directedReadOptions != null) { + SpannerUtil.verifyDirectedReadOptions(directedReadOptions); + } return new SpannerOptions(this); } @@ -1291,6 +1304,10 @@ public String getCompressorName() { return compressorName; } + public DirectedReadOptions getDirectedReadOptions() { + return directedReadOptions; + } + /** Returns the default query options to use for the specific database. */ public QueryOptions getDefaultQueryOptions(DatabaseId databaseId) { // Use the specific query options for the database if any have been specified. These have diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index 3d3b34c4c39..72ed378c3d9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -717,7 +717,7 @@ private ResultSet internalExecuteUpdate( /* withTransactionSelector = */ true); try { com.google.spanner.v1.ResultSet resultSet = - rpc.executeQuery(builder.build(), session.getOptions()); + rpc.executeQuery(builder.build(), session.getOptions(), false); if (resultSet.getMetadata().hasTransaction()) { onTransactionMetadata( resultSet.getMetadata().getTransaction(), builder.getTransaction().hasBegin()); @@ -747,7 +747,7 @@ public ApiFuture executeUpdateAsync(Statement statement, UpdateOption... o // Register the update as an async operation that must finish before the transaction may // commit. increaseAsyncOperations(); - resultSet = rpc.executeQueryAsync(builder.build(), session.getOptions()); + resultSet = rpc.executeQueryAsync(builder.build(), session.getOptions(), false); } catch (Throwable t) { decreaseAsyncOperations(); throw t; 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 7f325665542..2b421ab05a0 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 @@ -160,6 +160,7 @@ import com.google.spanner.v1.CommitResponse; import com.google.spanner.v1.CreateSessionRequest; import com.google.spanner.v1.DeleteSessionRequest; +import com.google.spanner.v1.DirectedReadOptions; import com.google.spanner.v1.ExecuteBatchDmlRequest; import com.google.spanner.v1.ExecuteBatchDmlResponse; import com.google.spanner.v1.ExecuteSqlRequest; @@ -304,6 +305,7 @@ private void awaitTermination() throws InterruptedException { private static final double ADMINISTRATIVE_REQUESTS_RATE_LIMIT = 1.0D; private static final ConcurrentMap ADMINISTRATIVE_REQUESTS_RATE_LIMITERS = new ConcurrentHashMap<>(); + private final DirectedReadOptions directedReadOptions; public static GapicSpannerRpc create(SpannerOptions options) { return new GapicSpannerRpc(options); @@ -354,6 +356,7 @@ public GapicSpannerRpc(final SpannerOptions options) { internalHeaderProviderBuilder.getResourceHeaderKey()); this.callCredentialsProvider = options.getCallCredentialsProvider(); this.compressorName = options.getCompressorName(); + this.directedReadOptions = options.getDirectedReadOptions(); if (initializeStubs) { // Create a managed executor provider. @@ -1636,7 +1639,11 @@ public ApiFuture asyncDeleteSession(String sessionName, @Nullable Map options) { + ReadRequest request, + ResultStreamConsumer consumer, + @Nullable Map options, + boolean readOnly) { + request = validateReadRequest(request, readOnly); GrpcCallContext context = newCallContext(options, request.getSession(), request, SpannerGrpc.getReadMethod()); SpannerResponseObserver responseObserver = new SpannerResponseObserver(consumer); @@ -1658,13 +1665,15 @@ public void cancel(String message) { } @Override - public ResultSet executeQuery(ExecuteSqlRequest request, @Nullable Map options) { - return get(executeQueryAsync(request, options)); + public ResultSet executeQuery( + ExecuteSqlRequest request, @Nullable Map options, boolean readOnly) { + return get(executeQueryAsync(request, options, readOnly)); } @Override public ApiFuture executeQueryAsync( - ExecuteSqlRequest request, @Nullable Map options) { + ExecuteSqlRequest request, @Nullable Map options, boolean readOnly) { + request = validateExecuteSqlRequest(request, readOnly); GrpcCallContext context = newCallContext(options, request.getSession(), request, SpannerGrpc.getExecuteSqlMethod()); return spannerStub.executeSqlCallable().futureCall(request, context); @@ -1673,6 +1682,7 @@ public ApiFuture executeQueryAsync( @Override public ResultSet executePartitionedDml( ExecuteSqlRequest request, @Nullable Map options) { + request = validateExecuteSqlRequest(request, false); GrpcCallContext context = newCallContext(options, request.getSession(), request, SpannerGrpc.getExecuteSqlMethod()); return get(partitionedDmlStub.executeSqlCallable().futureCall(request, context)); @@ -1686,6 +1696,7 @@ public RetrySettings getPartitionedDmlRetrySettings() { @Override public ServerStream executeStreamingPartitionedDml( ExecuteSqlRequest request, Map options, Duration timeout) { + request = validateExecuteSqlRequest(request, false); GrpcCallContext context = newCallContext( options, request.getSession(), request, SpannerGrpc.getExecuteStreamingSqlMethod()); @@ -1696,7 +1707,11 @@ public ServerStream executeStreamingPartitionedDml( @Override public StreamingCall executeQuery( - ExecuteSqlRequest request, ResultStreamConsumer consumer, @Nullable Map options) { + ExecuteSqlRequest request, + ResultStreamConsumer consumer, + @Nullable Map options, + boolean readOnly) { + request = validateExecuteSqlRequest(request, readOnly); GrpcCallContext context = newCallContext( options, request.getSession(), request, SpannerGrpc.getExecuteStreamingSqlMethod()); @@ -2014,4 +2029,32 @@ private static Duration systemProperty(String name, int defaultValue) { String stringValue = System.getProperty(name, ""); return Duration.ofSeconds(stringValue.isEmpty() ? defaultValue : Integer.parseInt(stringValue)); } + + private ExecuteSqlRequest validateExecuteSqlRequest(ExecuteSqlRequest request, boolean readOnly) { + if (!readOnly) { + if (request.hasDirectedReadOptions() || (directedReadOptions != null)) { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.FAILED_PRECONDITION, + "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); + } + } + if (directedReadOptions != null) { + return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); + } + return request; + } + + private ReadRequest validateReadRequest(ReadRequest request, boolean readOnly) { + if (!readOnly) { + if (request.hasDirectedReadOptions() || (directedReadOptions != null)) { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.FAILED_PRECONDITION, + "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); + } + } + if (directedReadOptions != null) { + return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); + } + return request; + } } 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 2f68b9c1df2..b5f7d86d912 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 @@ -320,12 +320,16 @@ ApiFuture asyncDeleteSession(String sessionName, @Nullable Map throws SpannerException; StreamingCall read( - ReadRequest request, ResultStreamConsumer consumer, @Nullable Map options); + ReadRequest request, + ResultStreamConsumer consumer, + @Nullable Map options, + boolean readOnly); - ResultSet executeQuery(ExecuteSqlRequest request, @Nullable Map options); + ResultSet executeQuery( + ExecuteSqlRequest request, @Nullable Map options, boolean readOnly); ApiFuture executeQueryAsync( - ExecuteSqlRequest request, @Nullable Map options); + ExecuteSqlRequest request, @Nullable Map options, boolean readOnly); ResultSet executePartitionedDml(ExecuteSqlRequest request, @Nullable Map options); @@ -335,7 +339,10 @@ ServerStream executeStreamingPartitionedDml( ExecuteSqlRequest request, @Nullable Map options, Duration timeout); StreamingCall executeQuery( - ExecuteSqlRequest request, ResultStreamConsumer consumer, @Nullable Map options); + ExecuteSqlRequest request, + ResultStreamConsumer consumer, + @Nullable Map options, + boolean readOnly); ExecuteBatchDmlResponse executeBatchDml(ExecuteBatchDmlRequest build, Map options); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java index 84f7fb0db9b..e6dd3a1a659 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java @@ -40,7 +40,6 @@ public void testCommitReturnsCommitStats() { new AsyncTransactionManagerImpl(session, mock(Span.class), Options.commitStats())) { when(session.newTransaction(Options.fromTransactionOptions(Options.commitStats()))) .thenReturn(transaction); - when(transaction.ensureTxnAsync()).thenReturn(ApiFutures.immediateFuture(null)); Timestamp commitTimestamp = Timestamp.ofTimeMicroseconds(1); CommitResponse response = mock(CommitResponse.class); when(response.getCommitTimestamp()).thenReturn(commitTimestamp); 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 1174cbf4ebd..23115b20ac4 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 @@ -414,7 +414,8 @@ public void request(int numMessages) {} private void mockRead(final PartialResultSet myResultSet) { final ArgumentCaptor consumer = ArgumentCaptor.forClass(SpannerRpc.ResultStreamConsumer.class); - Mockito.when(rpc.read(Mockito.any(), consumer.capture(), Mockito.eq(options))) + Mockito.when( + rpc.read(Mockito.any(), consumer.capture(), Mockito.eq(options), Mockito.anyBoolean())) .then( invocation -> { consumer.getValue().onPartialResultSet(myResultSet); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java index 4e5b1e0395c..666eb5f4130 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java @@ -763,9 +763,12 @@ public void testSessionNotFoundReadWriteTransaction() { when(rpc.asyncDeleteSession(Mockito.anyString(), Mockito.anyMap())) .thenReturn(ApiFutures.immediateFuture(Empty.getDefaultInstance())); when(rpc.executeQuery( - any(ExecuteSqlRequest.class), any(ResultStreamConsumer.class), any(Map.class))) + any(ExecuteSqlRequest.class), + any(ResultStreamConsumer.class), + any(Map.class), + any(Boolean.class))) .thenReturn(closedStreamingCall); - when(rpc.executeQuery(any(ExecuteSqlRequest.class), any(Map.class))) + when(rpc.executeQuery(any(ExecuteSqlRequest.class), any(Map.class), any(Boolean.class))) .thenThrow(sessionNotFound); when(rpc.executeBatchDml(any(ExecuteBatchDmlRequest.class), any(Map.class))) .thenThrow(sessionNotFound); 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 55df44a96df..e6c8a726506 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 @@ -286,7 +286,8 @@ public void inlineBegin() { .setId(ByteString.copyFromUtf8(UUID.randomUUID().toString())) .build())); final AtomicInteger transactionsStarted = new AtomicInteger(); - when(rpc.executeQuery(Mockito.any(ExecuteSqlRequest.class), Mockito.anyMap())) + when(rpc.executeQuery( + Mockito.any(ExecuteSqlRequest.class), Mockito.anyMap(), Mockito.anyBoolean())) .thenAnswer( invocation -> { ResultSet.Builder builder = @@ -334,7 +335,9 @@ public void inlineBegin() { verify(rpc, Mockito.never()) .beginTransaction(Mockito.any(BeginTransactionRequest.class), Mockito.anyMap()); // We should have 2 ExecuteSql requests. - verify(rpc, times(2)).executeQuery(Mockito.any(ExecuteSqlRequest.class), Mockito.anyMap()); + verify(rpc, times(2)) + .executeQuery( + Mockito.any(ExecuteSqlRequest.class), Mockito.anyMap(), Mockito.anyBoolean()); // But only 1 with a BeginTransaction. assertThat(transactionsStarted.get()).isEqualTo(1); } 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 04ac46d887b..b2ab77b5503 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 @@ -100,7 +100,8 @@ public void setUp() { MockitoAnnotations.initMocks(this); firstRun = true; when(session.newTransaction(Options.fromTransactionOptions())).thenReturn(txn); - when(rpc.executeQuery(Mockito.any(ExecuteSqlRequest.class), Mockito.anyMap())) + when(rpc.executeQuery( + Mockito.any(ExecuteSqlRequest.class), Mockito.anyMap(), Mockito.anyBoolean())) .thenAnswer( invocation -> { ResultSet.Builder builder = From 9adc89c4f7b7445f36c352963082815967db6a5c Mon Sep 17 00:00:00 2001 From: rajatrb Date: Mon, 10 Apr 2023 14:19:56 +0000 Subject: [PATCH 02/18] feat: add support for Directed Read options --- .../com/google/cloud/spanner/SpannerUtil.java | 28 +++++++ .../google/cloud/spanner/SpannerUtilTest.java | 76 +++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java new file mode 100644 index 00000000000..b6fd1746386 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java @@ -0,0 +1,28 @@ +package com.google.cloud.spanner; + +import com.google.spanner.v1.DirectedReadOptions; + +/** Utility methods for Spanner. */ +class SpannerUtil { + static final int MAX_REPLICA_SELECTIONS_COUNT = 10; + + static void verifyDirectedReadOptions(DirectedReadOptions directedReadOptions) { + if (directedReadOptions.hasIncludeReplicas() && directedReadOptions.hasExcludeReplicas()) { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.INVALID_ARGUMENT, + "Only one of include_replicas or exclude_replicas can be set"); + } + if ((directedReadOptions.hasIncludeReplicas() + && directedReadOptions.getIncludeReplicas().getReplicaSelectionsCount() + > MAX_REPLICA_SELECTIONS_COUNT) + || (directedReadOptions.hasExcludeReplicas() + && directedReadOptions.getExcludeReplicas().getReplicaSelectionsCount() + > MAX_REPLICA_SELECTIONS_COUNT)) { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.INVALID_ARGUMENT, + String.format( + "Maximum length of replica selection allowed in IncludeReplicas/ExcludeReplicas is %d", + MAX_REPLICA_SELECTIONS_COUNT)); + } + } +} diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java new file mode 100644 index 00000000000..becbd8e12ca --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java @@ -0,0 +1,76 @@ +package com.google.cloud.spanner; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import com.google.spanner.v1.DirectedReadOptions; +import com.google.spanner.v1.DirectedReadOptions.ExcludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link com.google.cloud.spanner.SpannerUtil}. */ +@RunWith(JUnit4.class) +public class SpannerUtilTest { + private DirectedReadOptions getDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { + List replicaSelectionList = new ArrayList<>( + Collections.nCopies(SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, + ReplicaSelection.newBuilder().setLocation("us-west1").build())); + return + DirectedReadOptions + .newBuilder() + .setIncludeReplicas(IncludeReplicas.newBuilder().addAllReplicaSelections(replicaSelectionList)) + .build(); + } + + private DirectedReadOptions getDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax() { + List replicaSelectionList = new ArrayList<>( + Collections.nCopies(SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, + ReplicaSelection.newBuilder().setLocation("us-east1").build())); + return + DirectedReadOptions + .newBuilder() + .setExcludeReplicas(ExcludeReplicas.newBuilder().addAllReplicaSelections(replicaSelectionList)) + .build(); + } + + public DirectedReadOptions getDirectedReadOptions_BothIncludeExcludeReplicasSet() { + return DirectedReadOptions + .newBuilder() + .setExcludeReplicas(ExcludeReplicas.newBuilder().addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-east1").build()).build()) + .setIncludeReplicas(IncludeReplicas.newBuilder().addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build()).build()) + .build(); + } + + @Test + public void testDirectedReadOptions_BothIncludeExcludeReplicasSet() { + // This test can be skipped because using the proto object directly will handle this case + // automatically and will set only IncludeReplicas, if both IncludeReplicas and ExcludeReplicas + // are passed in. + DirectedReadOptions directedReadOptions = getDirectedReadOptions_BothIncludeExcludeReplicasSet(); + SpannerException e = assertThrows(SpannerException.class, () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); + assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); + } + + @Test + public void testDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { + DirectedReadOptions directedReadOptions = getDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax(); + SpannerException e = assertThrows(SpannerException.class, () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); + assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); + } + + @Test + public void testDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax() { + DirectedReadOptions directedReadOptions = getDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax(); + SpannerException e = assertThrows(SpannerException.class, () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); + assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); + } + +} From 2222367f5918f0a8096bd030c1b83c95ceb46989 Mon Sep 17 00:00:00 2001 From: rajatrb Date: Mon, 10 Apr 2023 14:21:00 +0000 Subject: [PATCH 03/18] feat: add support for Directed Read options --- .../google/cloud/spanner/SpannerUtilTest.java | 80 ++++++++++++------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java index becbd8e12ca..47d8ac7f5a1 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java @@ -17,35 +17,42 @@ /** Unit tests for {@link com.google.cloud.spanner.SpannerUtil}. */ @RunWith(JUnit4.class) public class SpannerUtilTest { - private DirectedReadOptions getDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { - List replicaSelectionList = new ArrayList<>( - Collections.nCopies(SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, - ReplicaSelection.newBuilder().setLocation("us-west1").build())); - return - DirectedReadOptions - .newBuilder() - .setIncludeReplicas(IncludeReplicas.newBuilder().addAllReplicaSelections(replicaSelectionList)) - .build(); + private DirectedReadOptions + getDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { + List replicaSelectionList = + new ArrayList<>( + Collections.nCopies( + SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, + ReplicaSelection.newBuilder().setLocation("us-west1").build())); + return DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder().addAllReplicaSelections(replicaSelectionList)) + .build(); } - private DirectedReadOptions getDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax() { - List replicaSelectionList = new ArrayList<>( - Collections.nCopies(SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, - ReplicaSelection.newBuilder().setLocation("us-east1").build())); - return - DirectedReadOptions - .newBuilder() - .setExcludeReplicas(ExcludeReplicas.newBuilder().addAllReplicaSelections(replicaSelectionList)) - .build(); + private DirectedReadOptions + getDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax() { + List replicaSelectionList = + new ArrayList<>( + Collections.nCopies( + SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, + ReplicaSelection.newBuilder().setLocation("us-east1").build())); + return DirectedReadOptions.newBuilder() + .setExcludeReplicas( + ExcludeReplicas.newBuilder().addAllReplicaSelections(replicaSelectionList)) + .build(); } public DirectedReadOptions getDirectedReadOptions_BothIncludeExcludeReplicasSet() { - return DirectedReadOptions - .newBuilder() - .setExcludeReplicas(ExcludeReplicas.newBuilder().addReplicaSelections( - ReplicaSelection.newBuilder().setLocation("us-east1").build()).build()) - .setIncludeReplicas(IncludeReplicas.newBuilder().addReplicaSelections( - ReplicaSelection.newBuilder().setLocation("us-west1").build()).build()) + return DirectedReadOptions.newBuilder() + .setExcludeReplicas( + ExcludeReplicas.newBuilder() + .addReplicaSelections(ReplicaSelection.newBuilder().setLocation("us-east1").build()) + .build()) + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections(ReplicaSelection.newBuilder().setLocation("us-west1").build()) + .build()) .build(); } @@ -54,23 +61,34 @@ public void testDirectedReadOptions_BothIncludeExcludeReplicasSet() { // This test can be skipped because using the proto object directly will handle this case // automatically and will set only IncludeReplicas, if both IncludeReplicas and ExcludeReplicas // are passed in. - DirectedReadOptions directedReadOptions = getDirectedReadOptions_BothIncludeExcludeReplicasSet(); - SpannerException e = assertThrows(SpannerException.class, () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); + DirectedReadOptions directedReadOptions = + getDirectedReadOptions_BothIncludeExcludeReplicasSet(); + SpannerException e = + assertThrows( + SpannerException.class, + () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } @Test public void testDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { - DirectedReadOptions directedReadOptions = getDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax(); - SpannerException e = assertThrows(SpannerException.class, () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); + DirectedReadOptions directedReadOptions = + getDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax(); + SpannerException e = + assertThrows( + SpannerException.class, + () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } @Test public void testDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax() { - DirectedReadOptions directedReadOptions = getDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax(); - SpannerException e = assertThrows(SpannerException.class, () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); + DirectedReadOptions directedReadOptions = + getDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterThanMax(); + SpannerException e = + assertThrows( + SpannerException.class, + () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } - } From 4902d7a42f26c34642c7bf7bc8753b6c16928282 Mon Sep 17 00:00:00 2001 From: rajatrb Date: Mon, 10 Apr 2023 15:41:20 +0000 Subject: [PATCH 04/18] add unit tests for Options --- .../com/google/cloud/spanner/Options.java | 4 +++ .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 4 +-- .../com/google/cloud/spanner/OptionsTest.java | 28 ++++++++++++++----- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index 99ee2277bfa..9f70e51e44a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -228,6 +228,10 @@ public static CreateUpdateDeleteAdminApiOption validateOnly(Boolean validateOnly return new ValidateOnlyOption(validateOnly); } + public static DirectedReadOption directedReadOption(DirectedReadOptions directedReadOptions) { + return new DirectedReadOption(directedReadOptions); + } + /** Option to request {@link CommitStats} for read/write transactions. */ static final class CommitStatsOption extends InternalOption implements TransactionOption { @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 2b421ab05a0..2e679f03f94 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 @@ -2038,7 +2038,7 @@ private ExecuteSqlRequest validateExecuteSqlRequest(ExecuteSqlRequest request, b "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); } } - if (directedReadOptions != null) { + if (!request.hasDirectedReadOptions() && directedReadOptions != null) { return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); } return request; @@ -2052,7 +2052,7 @@ private ReadRequest validateReadRequest(ReadRequest request, boolean readOnly) { "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); } } - if (directedReadOptions != null) { + if (!request.hasDirectedReadOptions() && directedReadOptions != null) { return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); } return request; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java index d40f9b39ea1..8fe77745ece 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java @@ -18,12 +18,16 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import com.google.cloud.spanner.Options.RpcPriority; +import com.google.spanner.v1.DirectedReadOptions; +import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; import com.google.spanner.v1.RequestOptions.Priority; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,7 +36,12 @@ /** Unit tests for {@link Options}. */ @RunWith(JUnit4.class) public class OptionsTest { - + private static final DirectedReadOptions DIRECTED_READ_OPTIONS = + DirectedReadOptions + .newBuilder() + .setIncludeReplicas(IncludeReplicas.newBuilder().addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build())) + .build(); @Test public void negativeLimitsNotAllowed() { IllegalArgumentException e = @@ -65,13 +74,14 @@ public void zeroPrefetchChunksNotAllowed() { public void allOptionsPresent() { Options options = Options.fromReadOptions( - Options.limit(10), Options.prefetchChunks(1), Options.dataBoostEnabled(true)); + Options.limit(10), Options.prefetchChunks(1), Options.dataBoostEnabled(true), Options.directedReadOption(DIRECTED_READ_OPTIONS)); assertThat(options.hasLimit()).isTrue(); assertThat(options.limit()).isEqualTo(10); assertThat(options.hasPrefetchChunks()).isTrue(); assertThat(options.prefetchChunks()).isEqualTo(1); assertThat(options.hasDataBoostEnabled()).isTrue(); assertTrue(options.dataBoostEnabled()); + assertEquals(options.directedReadOptions(), DIRECTED_READ_OPTIONS); } @Test @@ -84,6 +94,7 @@ public void allOptionsAbsent() { assertThat(options.hasPriority()).isFalse(); assertThat(options.hasTag()).isFalse(); assertThat(options.hasDataBoostEnabled()).isFalse(); + assertFalse(options.hasDirectedReadOptions()); assertThat(options.toString()).isEqualTo(""); assertThat(options.equals(options)).isTrue(); assertThat(options.equals(null)).isFalse(); @@ -161,14 +172,14 @@ public void readOptionsTest() { boolean dataBoost = true; Options options = Options.fromReadOptions( - Options.limit(limit), Options.tag(tag), Options.dataBoostEnabled(true)); + Options.limit(limit), Options.tag(tag), Options.dataBoostEnabled(true), Options.directedReadOption(DIRECTED_READ_OPTIONS)); assertThat(options.toString()) .isEqualTo( - "limit: " + limit + " " + "tag: " + tag + " " + "dataBoostEnabled: " + dataBoost + " "); + "limit: " + limit + " " + "tag: " + tag + " " + "dataBoostEnabled: " + dataBoost + " " + "directedReadOptions: " + DIRECTED_READ_OPTIONS + " "); assertThat(options.tag()).isEqualTo(tag); assertEquals(dataBoost, options.dataBoostEnabled()); - assertThat(options.hashCode()).isEqualTo(-96091607); + assertEquals(DIRECTED_READ_OPTIONS, options.directedReadOptions()); } @Test @@ -199,7 +210,7 @@ public void queryOptionsTest() { boolean dataBoost = true; Options options = Options.fromQueryOptions( - Options.prefetchChunks(chunks), Options.tag(tag), Options.dataBoostEnabled(true)); + Options.prefetchChunks(chunks), Options.tag(tag), Options.dataBoostEnabled(true), Options.directedReadOption(DIRECTED_READ_OPTIONS)); assertThat(options.toString()) .isEqualTo( "prefetchChunks: " @@ -210,11 +221,14 @@ public void queryOptionsTest() { + " " + "dataBoostEnabled: " + dataBoost + + " " + + "directedReadOptions: " + + DIRECTED_READ_OPTIONS + " "); assertThat(options.prefetchChunks()).isEqualTo(chunks); assertThat(options.tag()).isEqualTo(tag); assertEquals(dataBoost, options.dataBoostEnabled()); - assertThat(options.hashCode()).isEqualTo(1274581983); + assertEquals(DIRECTED_READ_OPTIONS, options.directedReadOptions()); } @Test From 1c51305f5425af6e79df6aa6da6703af654837fb Mon Sep 17 00:00:00 2001 From: rajatrb Date: Mon, 10 Apr 2023 15:42:04 +0000 Subject: [PATCH 05/18] feat: add support for Directed Read options --- .../com/google/cloud/spanner/OptionsTest.java | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java index 8fe77745ece..a161f1d5067 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java @@ -37,11 +37,13 @@ @RunWith(JUnit4.class) public class OptionsTest { private static final DirectedReadOptions DIRECTED_READ_OPTIONS = - DirectedReadOptions - .newBuilder() - .setIncludeReplicas(IncludeReplicas.newBuilder().addReplicaSelections( - ReplicaSelection.newBuilder().setLocation("us-west1").build())) + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build())) .build(); + @Test public void negativeLimitsNotAllowed() { IllegalArgumentException e = @@ -74,7 +76,10 @@ public void zeroPrefetchChunksNotAllowed() { public void allOptionsPresent() { Options options = Options.fromReadOptions( - Options.limit(10), Options.prefetchChunks(1), Options.dataBoostEnabled(true), Options.directedReadOption(DIRECTED_READ_OPTIONS)); + Options.limit(10), + Options.prefetchChunks(1), + Options.dataBoostEnabled(true), + Options.directedReadOption(DIRECTED_READ_OPTIONS)); assertThat(options.hasLimit()).isTrue(); assertThat(options.limit()).isEqualTo(10); assertThat(options.hasPrefetchChunks()).isTrue(); @@ -172,11 +177,25 @@ public void readOptionsTest() { boolean dataBoost = true; Options options = Options.fromReadOptions( - Options.limit(limit), Options.tag(tag), Options.dataBoostEnabled(true), Options.directedReadOption(DIRECTED_READ_OPTIONS)); + Options.limit(limit), + Options.tag(tag), + Options.dataBoostEnabled(true), + Options.directedReadOption(DIRECTED_READ_OPTIONS)); assertThat(options.toString()) .isEqualTo( - "limit: " + limit + " " + "tag: " + tag + " " + "dataBoostEnabled: " + dataBoost + " " + "directedReadOptions: " + DIRECTED_READ_OPTIONS + " "); + "limit: " + + limit + + " " + + "tag: " + + tag + + " " + + "dataBoostEnabled: " + + dataBoost + + " " + + "directedReadOptions: " + + DIRECTED_READ_OPTIONS + + " "); assertThat(options.tag()).isEqualTo(tag); assertEquals(dataBoost, options.dataBoostEnabled()); assertEquals(DIRECTED_READ_OPTIONS, options.directedReadOptions()); @@ -210,7 +229,10 @@ public void queryOptionsTest() { boolean dataBoost = true; Options options = Options.fromQueryOptions( - Options.prefetchChunks(chunks), Options.tag(tag), Options.dataBoostEnabled(true), Options.directedReadOption(DIRECTED_READ_OPTIONS)); + Options.prefetchChunks(chunks), + Options.tag(tag), + Options.dataBoostEnabled(true), + Options.directedReadOption(DIRECTED_READ_OPTIONS)); assertThat(options.toString()) .isEqualTo( "prefetchChunks: " From d26ddcea5504785a7a386abcdb8fb2de7466ba14 Mon Sep 17 00:00:00 2001 From: rajatrb Date: Thu, 20 Apr 2023 16:40:42 +0000 Subject: [PATCH 06/18] add exhaustive unit tests and integration test. --- .../cloud/spanner/AbstractReadContext.java | 8 +- .../com/google/cloud/spanner/Options.java | 4 +- .../cloud/spanner/TransactionRunnerImpl.java | 5 +- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 5 +- .../spanner/AbstractReadContextTest.java | 23 +++ .../cloud/spanner/DatabaseClientImplTest.java | 184 ++++++++++++++++++ .../com/google/cloud/spanner/OptionsTest.java | 6 +- .../google/cloud/spanner/SpannerUtilTest.java | 14 -- .../google/cloud/spanner/it/ITReadTest.java | 34 ++++ .../spanner/spi/v1/GapicSpannerRpcTest.java | 150 ++++++++++++++ 10 files changed, 408 insertions(+), 25 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index 3270a8f40b4..8d05d86b471 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -387,6 +387,8 @@ void initTransaction() { @GuardedBy("lock") protected boolean isClosed = false; + protected boolean readOnly; + // A per-transaction sequence number used to identify this ExecuteSqlRequests. Required for DML, // ignored for query by the server. private AtomicLong seqNo = new AtomicLong(); @@ -405,6 +407,7 @@ void initTransaction() { this.defaultQueryOptions = builder.defaultQueryOptions; this.span = builder.span; this.executorProvider = builder.executorProvider; + this.readOnly = true; } @Override @@ -671,7 +674,8 @@ CloseableIterator startStream(@Nullable ByteString resumeToken request.setTransaction(selector); } SpannerRpc.StreamingCall call = - rpc.executeQuery(request.build(), stream.consumer(), session.getOptions(), true); + rpc.executeQuery( + request.build(), stream.consumer(), session.getOptions(), readOnly); call.request(prefetchChunks); stream.setCall(call, request.getTransaction().hasBegin()); return stream; @@ -806,7 +810,7 @@ CloseableIterator startStream(@Nullable ByteString resumeToken } builder.setRequestOptions(buildRequestOptions(readOptions)); SpannerRpc.StreamingCall call = - rpc.read(builder.build(), stream.consumer(), session.getOptions(), true); + rpc.read(builder.build(), stream.consumer(), session.getOptions(), readOnly); call.request(prefetchChunks); stream.setCall(call, /* withBeginTransaction = */ builder.getTransaction().hasBegin()); return stream; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index 9f70e51e44a..15d2cd6ba11 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -162,7 +162,7 @@ public static ListOption pageSize(int pageSize) { * is not generally available now). */ @BetaApi - public static DataBoostQueryOption dataBoostEnabled(Boolean dataBoostEnabled) { + public static ReadAndQueryOption dataBoostEnabled(Boolean dataBoostEnabled) { return new DataBoostQueryOption(dataBoostEnabled); } @@ -228,7 +228,7 @@ public static CreateUpdateDeleteAdminApiOption validateOnly(Boolean validateOnly return new ValidateOnlyOption(validateOnly); } - public static DirectedReadOption directedReadOption(DirectedReadOptions directedReadOptions) { + public static ReadAndQueryOption directedRead(DirectedReadOptions directedReadOptions) { return new DirectedReadOption(directedReadOptions); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index 72ed378c3d9..22a678108c5 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -196,6 +196,7 @@ private TransactionContextImpl(Builder builder) { this.trackTransactionStarter = builder.trackTransactionStarter; this.options = builder.options; this.finishedAsyncOperations.set(null); + this.readOnly = false; } private void increaseAsyncOperations() { @@ -717,7 +718,7 @@ private ResultSet internalExecuteUpdate( /* withTransactionSelector = */ true); try { com.google.spanner.v1.ResultSet resultSet = - rpc.executeQuery(builder.build(), session.getOptions(), false); + rpc.executeQuery(builder.build(), session.getOptions(), readOnly); if (resultSet.getMetadata().hasTransaction()) { onTransactionMetadata( resultSet.getMetadata().getTransaction(), builder.getTransaction().hasBegin()); @@ -747,7 +748,7 @@ public ApiFuture executeUpdateAsync(Statement statement, UpdateOption... o // Register the update as an async operation that must finish before the transaction may // commit. increaseAsyncOperations(); - resultSet = rpc.executeQueryAsync(builder.build(), session.getOptions(), false); + resultSet = rpc.executeQueryAsync(builder.build(), session.getOptions(), readOnly); } catch (Throwable t) { decreaseAsyncOperations(); throw t; 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 2e679f03f94..3dfd2ee0fe9 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 @@ -2030,7 +2030,8 @@ private static Duration systemProperty(String name, int defaultValue) { return Duration.ofSeconds(stringValue.isEmpty() ? defaultValue : Integer.parseInt(stringValue)); } - private ExecuteSqlRequest validateExecuteSqlRequest(ExecuteSqlRequest request, boolean readOnly) { + @VisibleForTesting + ExecuteSqlRequest validateExecuteSqlRequest(ExecuteSqlRequest request, boolean readOnly) { if (!readOnly) { if (request.hasDirectedReadOptions() || (directedReadOptions != null)) { throw SpannerExceptionFactory.newSpannerException( @@ -2044,7 +2045,7 @@ private ExecuteSqlRequest validateExecuteSqlRequest(ExecuteSqlRequest request, b return request; } - private ReadRequest validateReadRequest(ReadRequest request, boolean readOnly) { + ReadRequest validateReadRequest(ReadRequest request, boolean readOnly) { if (!readOnly) { if (request.hasDirectedReadOptions() || (directedReadOptions != null)) { throw SpannerExceptionFactory.newSpannerException( diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java index 31b73581f6b..a54189a2918 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java @@ -25,6 +25,9 @@ import com.google.api.gax.core.ExecutorProvider; import com.google.cloud.spanner.Options.RpcPriority; import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.spanner.v1.DirectedReadOptions; +import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; import com.google.spanner.v1.ExecuteBatchDmlRequest; import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.ExecuteSqlRequest.QueryMode; @@ -45,6 +48,14 @@ @RunWith(Parameterized.class) public class AbstractReadContextTest { + private static final DirectedReadOptions DIRECTED_READ_OPTIONS = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build())) + .build(); + @Parameter(0) public QueryOptions defaultQueryOptions; @@ -200,6 +211,17 @@ public void testGetExecuteSqlRequestBuilderWithDataBoost() { assertTrue(request.getDataBoostEnabled()); } + @Test + public void testGetExecuteSqlRequestBuilderWithDirectedReadOptions() { + ExecuteSqlRequest.Builder request = + context.getExecuteSqlRequestBuilder( + Statement.of("SELECT * FROM FOO"), + QueryMode.NORMAL, + Options.fromQueryOptions(Options.directedRead(DIRECTED_READ_OPTIONS)), + false); + assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS); + } + @Test public void testGetExecuteBatchDmlRequestBuilderWithPriority() { ExecuteBatchDmlRequest.Builder request = @@ -209,6 +231,7 @@ public void testGetExecuteBatchDmlRequestBuilderWithPriority() { assertEquals(Priority.PRIORITY_LOW, request.getRequestOptions().getPriority()); } + @Test public void executeSqlRequestBuilderWithRequestOptions() { ExecuteSqlRequest request = context diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index 0b88edc7f69..182c1a35d16 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -63,6 +63,9 @@ import com.google.protobuf.NullValue; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.DeleteSessionRequest; +import com.google.spanner.v1.DirectedReadOptions; +import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; import com.google.spanner.v1.ExecuteBatchDmlRequest; import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.ExecuteSqlRequest.QueryMode; @@ -131,6 +134,20 @@ public class DatabaseClientImplTest { private Spanner spanner; private Spanner spannerWithEmptySessionPool; private static ExecutorService executor; + private static final DirectedReadOptions DIRECTED_READ_OPTIONS1 = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build())) + .build(); + private static final DirectedReadOptions DIRECTED_READ_OPTIONS2 = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-east1").build())) + .build(); @BeforeClass public static void startStaticServer() throws IOException { @@ -332,6 +349,45 @@ public void testExecuteQueryWithTag() { assertThat(request.getRequestOptions().getTransactionTag()).isEmpty(); } + @Test + public void testExecuteQueryWithDirectedReadOptionsViaRequest() { + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + try (ResultSet resultSet = + client.singleUse().executeQuery(SELECT1, Options.directedRead(DIRECTED_READ_OPTIONS1))) { + while (resultSet.next()) {} + } + + List requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class); + assertThat(requests).hasSize(1); + ExecuteSqlRequest request = requests.get(0); + assertTrue(request.hasDirectedReadOptions()); + assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); + } + + @Test + public void testExecuteQueryWithDirectedReadOptionsViaSpannerOptions() { + Spanner spannerWithDirectedReadOptions = + spanner + .getOptions() + .toBuilder() + .setDirectedReadOptions(DIRECTED_READ_OPTIONS2) + .build() + .getService(); + DatabaseClient client = + spannerWithDirectedReadOptions.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + try (ResultSet resultSet = client.singleUse().executeQuery(SELECT1)) { + while (resultSet.next()) {} + } + + List requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class); + assertThat(requests).hasSize(1); + ExecuteSqlRequest request = requests.get(0); + assertTrue(request.hasDirectedReadOptions()); + assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); + } + @Test public void testExecuteReadWithTag() { DatabaseClient client = @@ -356,6 +412,52 @@ public void testExecuteReadWithTag() { assertThat(request.getRequestOptions().getTransactionTag()).isEmpty(); } + @Test + public void testExecuteReadWithDirectedReadOptionsViaRequest() { + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + try (ResultSet resultSet = + client + .singleUse() + .read( + READ_TABLE_NAME, + KeySet.singleKey(Key.of(1L)), + READ_COLUMN_NAMES, + Options.directedRead(DIRECTED_READ_OPTIONS1))) { + while (resultSet.next()) {} + } + + List requests = mockSpanner.getRequestsOfType(ReadRequest.class); + assertThat(requests).hasSize(1); + ReadRequest request = requests.get(0); + assertTrue(request.hasDirectedReadOptions()); + assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); + } + + @Test + public void testExecuteReadWithDirectedReadOptionsViaSpannerOptions() { + Spanner spannerWithDirectedReadOptions = + spanner + .getOptions() + .toBuilder() + .setDirectedReadOptions(DIRECTED_READ_OPTIONS2) + .build() + .getService(); + DatabaseClient client = + spannerWithDirectedReadOptions.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + try (ResultSet resultSet = + client.singleUse().read(READ_TABLE_NAME, KeySet.singleKey(Key.of(1L)), READ_COLUMN_NAMES)) { + while (resultSet.next()) {} + } + + List requests = mockSpanner.getRequestsOfType(ReadRequest.class); + assertThat(requests).hasSize(1); + ReadRequest request = requests.get(0); + assertTrue(request.hasDirectedReadOptions()); + assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); + } + @Test public void testReadWriteExecuteQueryWithTag() { DatabaseClient client = @@ -381,6 +483,60 @@ public void testReadWriteExecuteQueryWithTag() { .isEqualTo("app=spanner,env=test,action=txn"); } + @Test + public void testReadWriteExecuteQueryWithDirectedReadOptionsViaRequest() { + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + TransactionRunner runner = client.readWriteTransaction(); + SpannerException e = + assertThrows( + SpannerException.class, + () -> + runner.run( + transaction -> { + try (ResultSet resultSet = + transaction.executeQuery( + SELECT1, Options.directedRead(DIRECTED_READ_OPTIONS1))) { + while (resultSet.next()) {} + } + return null; + })); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + + List requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class); + assertThat(requests).hasSize(0); + } + + @Test + public void testReadWriteExecuteQueryWithDirectedReadOptionsViaSpannerOptions() { + Spanner spannerWithDirectedReadOptions = + spanner + .getOptions() + .toBuilder() + .setDirectedReadOptions(DIRECTED_READ_OPTIONS2) + .build() + .getService(); + DatabaseClient client = + spannerWithDirectedReadOptions.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + TransactionRunner runner = client.readWriteTransaction(); + SpannerException e = + assertThrows( + SpannerException.class, + () -> + runner.run( + transaction -> { + try (ResultSet resultSet = transaction.executeQuery(SELECT1)) { + while (resultSet.next()) {} + } + return null; + })); + + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + List requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class); + assertEquals(requests.size(), 0); + } + @Test public void testReadWriteExecuteReadWithTag() { DatabaseClient client = @@ -410,6 +566,34 @@ public void testReadWriteExecuteReadWithTag() { .isEqualTo("app=spanner,env=test,action=txn"); } + // FIX_IT + @Test + public void testReadWriteExecuteReadWithDirectedReadOptionsViaRequest() { + DatabaseClient client = + spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + TransactionRunner runner = client.readWriteTransaction(); + SpannerException e = + assertThrows( + SpannerException.class, + () -> + runner.run( + transaction -> { + try (ResultSet resultSet = + transaction.read( + READ_TABLE_NAME, + KeySet.singleKey(Key.of(1L)), + READ_COLUMN_NAMES, + Options.directedRead(DIRECTED_READ_OPTIONS1))) { + while (resultSet.next()) {} + } + return null; + })); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + + List requests = mockSpanner.getRequestsOfType(ReadRequest.class); + assertEquals(requests.size(), 0); + } + @Test public void testExecuteUpdateWithTag() { DatabaseClient client = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java index a161f1d5067..ef802030f09 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java @@ -79,7 +79,7 @@ public void allOptionsPresent() { Options.limit(10), Options.prefetchChunks(1), Options.dataBoostEnabled(true), - Options.directedReadOption(DIRECTED_READ_OPTIONS)); + Options.directedRead(DIRECTED_READ_OPTIONS)); assertThat(options.hasLimit()).isTrue(); assertThat(options.limit()).isEqualTo(10); assertThat(options.hasPrefetchChunks()).isTrue(); @@ -180,7 +180,7 @@ public void readOptionsTest() { Options.limit(limit), Options.tag(tag), Options.dataBoostEnabled(true), - Options.directedReadOption(DIRECTED_READ_OPTIONS)); + Options.directedRead(DIRECTED_READ_OPTIONS)); assertThat(options.toString()) .isEqualTo( @@ -232,7 +232,7 @@ public void queryOptionsTest() { Options.prefetchChunks(chunks), Options.tag(tag), Options.dataBoostEnabled(true), - Options.directedReadOption(DIRECTED_READ_OPTIONS)); + Options.directedRead(DIRECTED_READ_OPTIONS)); assertThat(options.toString()) .isEqualTo( "prefetchChunks: " diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java index 47d8ac7f5a1..e2d860274b9 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java @@ -56,20 +56,6 @@ public DirectedReadOptions getDirectedReadOptions_BothIncludeExcludeReplicasSet( .build(); } - @Test - public void testDirectedReadOptions_BothIncludeExcludeReplicasSet() { - // This test can be skipped because using the proto object directly will handle this case - // automatically and will set only IncludeReplicas, if both IncludeReplicas and ExcludeReplicas - // are passed in. - DirectedReadOptions directedReadOptions = - getDirectedReadOptions_BothIncludeExcludeReplicasSet(); - SpannerException e = - assertThrows( - SpannerException.class, - () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); - assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); - } - @Test public void testDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { DirectedReadOptions directedReadOptions = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java index c28b48c529a..f347c767a20 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java @@ -21,6 +21,9 @@ import static com.google.cloud.spanner.testing.EmulatorSpannerHelper.isUsingEmulator; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.cloud.spanner.Database; @@ -42,6 +45,9 @@ import com.google.cloud.spanner.Type; import com.google.cloud.spanner.connection.ConnectionOptions; import com.google.cloud.spanner.testing.RemoteSpannerHelper; +import com.google.spanner.v1.DirectedReadOptions; +import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; import io.grpc.Context; import java.util.ArrayList; import java.util.Arrays; @@ -309,6 +315,34 @@ public void indexMultiPointRead() { checkRange(Source.INDEX, keys, 3, 5, 7); } + @Test + public void pointReadWithDirectedReadOptions() { + DirectedReadOptions directedReadOptions = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder() + .setLocation("us-west1") + .setType(ReplicaSelection.Type.READ_ONLY) + .build()) + .setAutoFailover(true)) + .build(); + try (ResultSet rs = + getClient(dialect.dialect) + .singleUse() + .read( + TABLE_NAME, + KeySet.singleKey(Key.of("k1")), + ALL_COLUMNS, + Options.directedRead(directedReadOptions))) { + assertTrue(rs.next()); + assertEquals(rs.getString(0), "k1"); + assertEquals(rs.getString(1), "v1"); + assertFalse(rs.next()); + } + } + @Test public void rowsAreSnapshots() { List rows = new ArrayList<>(); 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 23ec9c682c8..11ee4af3cf2 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 @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assume.assumeTrue; @@ -46,8 +47,12 @@ import com.google.cloud.spanner.spi.v1.SpannerRpc.Option; import com.google.protobuf.ListValue; import com.google.rpc.ErrorInfo; +import com.google.spanner.v1.DirectedReadOptions; +import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.GetSessionRequest; +import com.google.spanner.v1.ReadRequest; import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.SpannerGrpc; import com.google.spanner.v1.StructType; @@ -128,6 +133,24 @@ public class GapicSpannerRpcTest { VARIABLE_OAUTH_TOKEN, new java.util.Date( System.currentTimeMillis() + TimeUnit.MILLISECONDS.convert(1L, TimeUnit.DAYS)))); + private static final DirectedReadOptions DIRECTED_READ_OPTIONS1 = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build())) + .build(); + private static final DirectedReadOptions DIRECTED_READ_OPTIONS2 = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-east1").build())) + .build(); + private static SpannerOptions OPTIONS_WITHOUT_DIRECTED_READ_OPTIONS; + private static SpannerOptions OPTIONS_WITH_DIRECTED_READ_OPTIONS; + private static GapicSpannerRpc RPC_WITH_DIRECTED_READ_OPTIONS; + private static GapicSpannerRpc RPC_WITHOUT_DIRECTED_READ_OPTIONS; private static MockSpannerServiceImpl mockSpanner; private static Server server; @@ -180,6 +203,15 @@ public ServerCall.Listener interceptCall( .start(); optionsMap.put(Option.CHANNEL_HINT, 1L); spanner = createSpannerOptions().getService(); + OPTIONS_WITHOUT_DIRECTED_READ_OPTIONS = createSpannerOptions(); + OPTIONS_WITH_DIRECTED_READ_OPTIONS = + OPTIONS_WITHOUT_DIRECTED_READ_OPTIONS + .toBuilder() + .setDirectedReadOptions(DIRECTED_READ_OPTIONS1) + .build(); + RPC_WITH_DIRECTED_READ_OPTIONS = new GapicSpannerRpc(OPTIONS_WITH_DIRECTED_READ_OPTIONS, false); + RPC_WITHOUT_DIRECTED_READ_OPTIONS = + new GapicSpannerRpc(OPTIONS_WITHOUT_DIRECTED_READ_OPTIONS, false); } @After @@ -449,6 +481,124 @@ public void testCustomUserAgent() { } } + @Test + public void testValidateExecuteSqlRequest() { + ExecuteSqlRequest.Builder requestBuilder = + ExecuteSqlRequest.newBuilder().setSql("SELECT * FROM FOO"); + ExecuteSqlRequest requestWithoutDirectedReadOptions = requestBuilder.build(); + ExecuteSqlRequest requestWithDirectedReadOptions = + requestBuilder.setDirectedReadOptions(DIRECTED_READ_OPTIONS2).build(); + // Case 1: Read-only transaction. + // Case 1.1: DirectedReadOptions passed in via SpannerOptions only. + ExecuteSqlRequest returnedRequest = + RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithoutDirectedReadOptions, true); + assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); + // Case 1.2: DirectedReadOptions passed in via ExecuteSqlRequest only. + returnedRequest = + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithDirectedReadOptions, true); + assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); + // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and ExecuteSqlRequest. + returnedRequest = + RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithDirectedReadOptions, true); + assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); + // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and ExecuteSqlRequest. + returnedRequest = + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithoutDirectedReadOptions, true); + assertFalse(returnedRequest.hasDirectedReadOptions()); + + // Case 2: Read-write/PDML transaction. + // Case 2.1: DirectedReadOptions passed in via SpannerOptions only. + SpannerException e = + assertThrows( + SpannerException.class, + () -> + RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithoutDirectedReadOptions, false)); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + // Case 2.2: DirectedReadOptions passed in via ExecuteSqlRequest only. + e = + assertThrows( + SpannerException.class, + () -> + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithDirectedReadOptions, false)); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and ExecuteSqlRequest. + e = + assertThrows( + SpannerException.class, + () -> + RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithDirectedReadOptions, false)); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and ExecuteSqlRequest. + returnedRequest = + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( + requestWithoutDirectedReadOptions, false); + assertFalse(returnedRequest.hasDirectedReadOptions()); + } + + @Test + public void testValidateReadRequest() { + ReadRequest.Builder requestBuilder = ReadRequest.newBuilder().setTable("tbl"); + ReadRequest requestWithoutDirectedReadOptions = requestBuilder.build(); + ReadRequest requestWithDirectedReadOptions = + requestBuilder.setDirectedReadOptions(DIRECTED_READ_OPTIONS2).build(); + // Case 1: Read-only transaction. + // Case 1.1: DirectedReadOptions passed in via SpannerOptions only. + ReadRequest returnedRequest = + RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithoutDirectedReadOptions, true); + assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); + // Case 1.2: DirectedReadOptions passed in via ReadRequest only. + returnedRequest = + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithDirectedReadOptions, true); + assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); + // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and ReadRequest. + returnedRequest = + RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithDirectedReadOptions, true); + assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); + // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and ReadRequest. + returnedRequest = + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( + requestWithoutDirectedReadOptions, true); + assertFalse(returnedRequest.hasDirectedReadOptions()); + + // Case 2: Read-write/PDML transaction. + // Case 2.1: DirectedReadOptions passed in via SpannerOptions only. + SpannerException e = + assertThrows( + SpannerException.class, + () -> + RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest( + requestWithoutDirectedReadOptions, false)); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + // Case 2.2: DirectedReadOptions passed in via ReadRequest only. + e = + assertThrows( + SpannerException.class, + () -> + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( + requestWithDirectedReadOptions, false)); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and ReadRequest. + e = + assertThrows( + SpannerException.class, + () -> + RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest( + requestWithDirectedReadOptions, false)); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and ReadRequest. + returnedRequest = + RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( + requestWithoutDirectedReadOptions, false); + assertFalse(returnedRequest.hasDirectedReadOptions()); + } + private SpannerOptions createSpannerOptions() { String endpoint = address.getHostString() + ":" + server.getPort(); return SpannerOptions.newBuilder() From 4e5e584f8c71833eaadba93deef578ef98bc258e Mon Sep 17 00:00:00 2001 From: rajatrb Date: Thu, 20 Apr 2023 17:18:36 +0000 Subject: [PATCH 07/18] refactoring --- .../com/google/cloud/spanner/SpannerUtil.java | 16 +++++ .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 8 +++ .../cloud/spanner/DatabaseClientImplTest.java | 9 ++- .../com/google/cloud/spanner/OptionsTest.java | 69 +++++++++---------- .../google/cloud/spanner/SpannerUtilTest.java | 29 ++++---- .../google/cloud/spanner/it/ITReadTest.java | 24 +++---- .../spanner/spi/v1/GapicSpannerRpcTest.java | 26 +++---- 7 files changed, 104 insertions(+), 77 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java index b6fd1746386..ed17bb4ea5d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java @@ -1,3 +1,19 @@ +/* + * Copyright 2023 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; import com.google.spanner.v1.DirectedReadOptions; 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 3dfd2ee0fe9..1db967fc02e 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 @@ -2039,6 +2039,10 @@ ExecuteSqlRequest validateExecuteSqlRequest(ExecuteSqlRequest request, boolean r "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); } } + // If DirectedReadOptions is not set at request-level, the request object won't be + // having DirectedReadOptions field set. Though, if DirectedReadOptions is set at client-level + // (through SpannerOptions), we must modify the request object to set the DirectedReadOptions + // proto field to this value. if (!request.hasDirectedReadOptions() && directedReadOptions != null) { return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); } @@ -2053,6 +2057,10 @@ ReadRequest validateReadRequest(ReadRequest request, boolean readOnly) { "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); } } + // If DirectedReadOptions is not set at request-level, the request object won't be + // having DirectedReadOptions field set. Though, if DirectedReadOptions is set at client-level + // (through SpannerOptions), we must modify the request object to set the DirectedReadOptions + // proto field to this value. if (!request.hasDirectedReadOptions() && directedReadOptions != null) { return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index 182c1a35d16..3aad0d98a7e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -359,7 +359,7 @@ public void testExecuteQueryWithDirectedReadOptionsViaRequest() { } List requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class); - assertThat(requests).hasSize(1); + assertEquals(requests.size(), 1); ExecuteSqlRequest request = requests.get(0); assertTrue(request.hasDirectedReadOptions()); assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); @@ -382,7 +382,7 @@ public void testExecuteQueryWithDirectedReadOptionsViaSpannerOptions() { } List requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class); - assertThat(requests).hasSize(1); + assertEquals(requests.size(), 1); ExecuteSqlRequest request = requests.get(0); assertTrue(request.hasDirectedReadOptions()); assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); @@ -428,7 +428,7 @@ public void testExecuteReadWithDirectedReadOptionsViaRequest() { } List requests = mockSpanner.getRequestsOfType(ReadRequest.class); - assertThat(requests).hasSize(1); + assertEquals(requests.size(), 1); ReadRequest request = requests.get(0); assertTrue(request.hasDirectedReadOptions()); assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); @@ -452,7 +452,7 @@ public void testExecuteReadWithDirectedReadOptionsViaSpannerOptions() { } List requests = mockSpanner.getRequestsOfType(ReadRequest.class); - assertThat(requests).hasSize(1); + assertEquals(requests.size(), 1); ReadRequest request = requests.get(0); assertTrue(request.hasDirectedReadOptions()); assertEquals(request.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); @@ -566,7 +566,6 @@ public void testReadWriteExecuteReadWithTag() { .isEqualTo("app=spanner,env=test,action=txn"); } - // FIX_IT @Test public void testReadWriteExecuteReadWithDirectedReadOptionsViaRequest() { DatabaseClient client = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java index ef802030f09..4ebbc75be10 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java @@ -80,32 +80,31 @@ public void allOptionsPresent() { Options.prefetchChunks(1), Options.dataBoostEnabled(true), Options.directedRead(DIRECTED_READ_OPTIONS)); - assertThat(options.hasLimit()).isTrue(); - assertThat(options.limit()).isEqualTo(10); - assertThat(options.hasPrefetchChunks()).isTrue(); - assertThat(options.prefetchChunks()).isEqualTo(1); - assertThat(options.hasDataBoostEnabled()).isTrue(); + assertTrue(options.hasLimit()); + assertEquals(options.limit(), 10); + assertTrue(options.hasPrefetchChunks()); + assertEquals(options.prefetchChunks(), 1); + assertTrue(options.hasDataBoostEnabled()); assertTrue(options.dataBoostEnabled()); + assertTrue(options.hasDirectedReadOptions()); assertEquals(options.directedReadOptions(), DIRECTED_READ_OPTIONS); } @Test public void allOptionsAbsent() { Options options = Options.fromReadOptions(); - assertThat(options.hasLimit()).isFalse(); - assertThat(options.hasPrefetchChunks()).isFalse(); - assertThat(options.hasFilter()).isFalse(); - assertThat(options.hasPageToken()).isFalse(); - assertThat(options.hasPriority()).isFalse(); - assertThat(options.hasTag()).isFalse(); - assertThat(options.hasDataBoostEnabled()).isFalse(); + assertFalse(options.hasLimit()); + assertFalse(options.hasPrefetchChunks()); + assertFalse(options.hasFilter()); + assertFalse(options.hasPageToken()); + assertFalse(options.hasPriority()); + assertFalse(options.hasTag()); + assertFalse(options.hasDataBoostEnabled()); assertFalse(options.hasDirectedReadOptions()); - assertThat(options.toString()).isEqualTo(""); - assertThat(options.equals(options)).isTrue(); - assertThat(options.equals(null)).isFalse(); - assertThat(options.equals(this)).isFalse(); - - assertThat(options.hashCode()).isEqualTo(31); + assertEquals(options.toString(), ""); + assertTrue(options.equals(options)); + assertNotEquals(null, options); + assertEquals(options.hashCode(), 31); } @Test @@ -196,7 +195,7 @@ public void readOptionsTest() { + "directedReadOptions: " + DIRECTED_READ_OPTIONS + " "); - assertThat(options.tag()).isEqualTo(tag); + assertEquals(options.tag(), tag); assertEquals(dataBoost, options.dataBoostEnabled()); assertEquals(DIRECTED_READ_OPTIONS, options.directedReadOptions()); } @@ -233,22 +232,22 @@ public void queryOptionsTest() { Options.tag(tag), Options.dataBoostEnabled(true), Options.directedRead(DIRECTED_READ_OPTIONS)); - assertThat(options.toString()) - .isEqualTo( - "prefetchChunks: " - + chunks - + " " - + "tag: " - + tag - + " " - + "dataBoostEnabled: " - + dataBoost - + " " - + "directedReadOptions: " - + DIRECTED_READ_OPTIONS - + " "); - assertThat(options.prefetchChunks()).isEqualTo(chunks); - assertThat(options.tag()).isEqualTo(tag); + assertEquals( + options.toString(), + "prefetchChunks: " + + chunks + + " " + + "tag: " + + tag + + " " + + "dataBoostEnabled: " + + dataBoost + + " " + + "directedReadOptions: " + + DIRECTED_READ_OPTIONS + + " "); + assertEquals(options.prefetchChunks(), chunks); + assertEquals(options.tag(), tag); assertEquals(dataBoost, options.dataBoostEnabled()); assertEquals(DIRECTED_READ_OPTIONS, options.directedReadOptions()); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java index e2d860274b9..4107306853b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java @@ -1,3 +1,19 @@ +/* + * Copyright 2023 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; import static org.junit.Assert.assertEquals; @@ -43,19 +59,6 @@ public class SpannerUtilTest { .build(); } - public DirectedReadOptions getDirectedReadOptions_BothIncludeExcludeReplicasSet() { - return DirectedReadOptions.newBuilder() - .setExcludeReplicas( - ExcludeReplicas.newBuilder() - .addReplicaSelections(ReplicaSelection.newBuilder().setLocation("us-east1").build()) - .build()) - .setIncludeReplicas( - IncludeReplicas.newBuilder() - .addReplicaSelections(ReplicaSelection.newBuilder().setLocation("us-west1").build()) - .build()) - .build(); - } - @Test public void testDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { DirectedReadOptions directedReadOptions = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java index f347c767a20..2263b732720 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITReadTest.java @@ -83,6 +83,17 @@ public class ITReadTest { private static final Type TABLE_TYPE = Type.struct( StructField.of("key", Type.string()), StructField.of("stringvalue", Type.string())); + private static DirectedReadOptions DIRECTED_READ_OPTIONS = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder() + .setLocation("us-west1") + .setType(ReplicaSelection.Type.READ_ONLY) + .build()) + .setAutoFailover(true)) + .build(); private static DatabaseClient googleStandardSQLClient; private static DatabaseClient postgreSQLClient; @@ -317,17 +328,6 @@ public void indexMultiPointRead() { @Test public void pointReadWithDirectedReadOptions() { - DirectedReadOptions directedReadOptions = - DirectedReadOptions.newBuilder() - .setIncludeReplicas( - IncludeReplicas.newBuilder() - .addReplicaSelections( - ReplicaSelection.newBuilder() - .setLocation("us-west1") - .setType(ReplicaSelection.Type.READ_ONLY) - .build()) - .setAutoFailover(true)) - .build(); try (ResultSet rs = getClient(dialect.dialect) .singleUse() @@ -335,7 +335,7 @@ public void pointReadWithDirectedReadOptions() { TABLE_NAME, KeySet.singleKey(Key.of("k1")), ALL_COLUMNS, - Options.directedRead(directedReadOptions))) { + Options.directedRead(DIRECTED_READ_OPTIONS))) { assertTrue(rs.next()); assertEquals(rs.getString(0), "k1"); assertEquals(rs.getString(1), "v1"); 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 11ee4af3cf2..1c71da67b1f 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 @@ -488,23 +488,24 @@ public void testValidateExecuteSqlRequest() { ExecuteSqlRequest requestWithoutDirectedReadOptions = requestBuilder.build(); ExecuteSqlRequest requestWithDirectedReadOptions = requestBuilder.setDirectedReadOptions(DIRECTED_READ_OPTIONS2).build(); + // Case 1: Read-only transaction. // Case 1.1: DirectedReadOptions passed in via SpannerOptions only. ExecuteSqlRequest returnedRequest = RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithoutDirectedReadOptions, true); assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); - // Case 1.2: DirectedReadOptions passed in via ExecuteSqlRequest only. + // Case 1.2: DirectedReadOptions passed in via request only. returnedRequest = RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithDirectedReadOptions, true); assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and ExecuteSqlRequest. + // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and request. returnedRequest = RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithDirectedReadOptions, true); assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and ExecuteSqlRequest. + // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and request. returnedRequest = RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithoutDirectedReadOptions, true); @@ -519,7 +520,7 @@ public void testValidateExecuteSqlRequest() { RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithoutDirectedReadOptions, false)); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.2: DirectedReadOptions passed in via ExecuteSqlRequest only. + // Case 2.2: DirectedReadOptions passed in via request only. e = assertThrows( SpannerException.class, @@ -527,7 +528,7 @@ public void testValidateExecuteSqlRequest() { RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithDirectedReadOptions, false)); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and ExecuteSqlRequest. + // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and request. e = assertThrows( SpannerException.class, @@ -535,7 +536,7 @@ public void testValidateExecuteSqlRequest() { RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithDirectedReadOptions, false)); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and ExecuteSqlRequest. + // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and request. returnedRequest = RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( requestWithoutDirectedReadOptions, false); @@ -548,20 +549,21 @@ public void testValidateReadRequest() { ReadRequest requestWithoutDirectedReadOptions = requestBuilder.build(); ReadRequest requestWithDirectedReadOptions = requestBuilder.setDirectedReadOptions(DIRECTED_READ_OPTIONS2).build(); + // Case 1: Read-only transaction. // Case 1.1: DirectedReadOptions passed in via SpannerOptions only. ReadRequest returnedRequest = RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithoutDirectedReadOptions, true); assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); - // Case 1.2: DirectedReadOptions passed in via ReadRequest only. + // Case 1.2: DirectedReadOptions passed in via request only. returnedRequest = RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithDirectedReadOptions, true); assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and ReadRequest. + // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and request. returnedRequest = RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithDirectedReadOptions, true); assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and ReadRequest. + // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and request. returnedRequest = RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( requestWithoutDirectedReadOptions, true); @@ -576,7 +578,7 @@ public void testValidateReadRequest() { RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest( requestWithoutDirectedReadOptions, false)); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.2: DirectedReadOptions passed in via ReadRequest only. + // Case 2.2: DirectedReadOptions passed in via request only. e = assertThrows( SpannerException.class, @@ -584,7 +586,7 @@ public void testValidateReadRequest() { RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( requestWithDirectedReadOptions, false)); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and ReadRequest. + // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and request. e = assertThrows( SpannerException.class, @@ -592,7 +594,7 @@ public void testValidateReadRequest() { RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest( requestWithDirectedReadOptions, false)); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and ReadRequest. + // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and request. returnedRequest = RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( requestWithoutDirectedReadOptions, false); From 279c108364ac225992ae18f457b3fc4553c52af7 Mon Sep 17 00:00:00 2001 From: rajatrb Date: Thu, 20 Apr 2023 17:40:34 +0000 Subject: [PATCH 08/18] add documentation --- .../main/java/com/google/cloud/spanner/Options.java | 1 + .../com/google/cloud/spanner/SpannerOptions.java | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index 15d2cd6ba11..fbb79f84753 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -228,6 +228,7 @@ public static CreateUpdateDeleteAdminApiOption validateOnly(Boolean validateOnly return new ValidateOnlyOption(validateOnly); } + /** Option to request DirectedRead for ReadOnlyTransaction and SingleUseTransaction. */ public static ReadAndQueryOption directedRead(DirectedReadOptions directedReadOptions) { return new DirectedReadOption(directedReadOptions); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index d30cb8c25bc..e558e6aff7d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1085,6 +1085,18 @@ public Builder setCompressorName(@Nullable String compressorName) { return this; } + /** + * Sets the {@link DirectedReadOptions} that specify which replicas or regions should be used + * for non-transactional reads or queries. + * + *

Note that DirectedReadOptions can only be set for ReadOnlyTransaction and + * SingleUseTransaction; it cannot be set for ReadWriteTransaction and + * PartitionedDmlTransaction. + * + *

DirectedReadOptions set at the request level will take precedence over the options set + * using this method. If DirectedReadOptions are set at the request level, they will override + * the options set here when the RPC call is made. + */ public Builder setDirectedReadOptions(DirectedReadOptions directedReadOptions) { Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); this.directedReadOptions = directedReadOptions; From c15ac9466626d1b1af1734c18d2dd3e68c0c29b3 Mon Sep 17 00:00:00 2001 From: rajatrb Date: Thu, 20 Apr 2023 17:44:29 +0000 Subject: [PATCH 09/18] add check to setter, instead of doing during build() --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index e558e6aff7d..1b2db590d2c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1099,6 +1099,7 @@ public Builder setCompressorName(@Nullable String compressorName) { */ public Builder setDirectedReadOptions(DirectedReadOptions directedReadOptions) { Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); + SpannerUtil.verifyDirectedReadOptions(directedReadOptions); this.directedReadOptions = directedReadOptions; return this; } @@ -1196,9 +1197,6 @@ public SpannerOptions build() { this.numChannels = this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS; } - if (directedReadOptions != null) { - SpannerUtil.verifyDirectedReadOptions(directedReadOptions); - } return new SpannerOptions(this); } From bda382a4e003733f78a7f6306e365e2a5ca61c4d Mon Sep 17 00:00:00 2001 From: rajatrb Date: Fri, 21 Apr 2023 09:47:33 +0000 Subject: [PATCH 10/18] refactoring to move validation and getting preferred DirectedReadOptions logic to SpannerUtil.java --- .../com/google/cloud/spanner/SpannerUtil.java | 25 +++- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 75 +++++------ .../google/cloud/spanner/SpannerUtilTest.java | 59 +++++++++ .../spanner/spi/v1/GapicSpannerRpcTest.java | 122 ------------------ 4 files changed, 115 insertions(+), 166 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java index ed17bb4ea5d..52ef8308a88 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java @@ -19,10 +19,10 @@ import com.google.spanner.v1.DirectedReadOptions; /** Utility methods for Spanner. */ -class SpannerUtil { +public class SpannerUtil { static final int MAX_REPLICA_SELECTIONS_COUNT = 10; - static void verifyDirectedReadOptions(DirectedReadOptions directedReadOptions) { + public static void verifyDirectedReadOptions(DirectedReadOptions directedReadOptions) { if (directedReadOptions.hasIncludeReplicas() && directedReadOptions.hasExcludeReplicas()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INVALID_ARGUMENT, @@ -41,4 +41,25 @@ static void verifyDirectedReadOptions(DirectedReadOptions directedReadOptions) { MAX_REPLICA_SELECTIONS_COUNT)); } } + + public static DirectedReadOptions validateAndGetPreferredDirectedReadOptions( + DirectedReadOptions directedReadOptionsForClient, + DirectedReadOptions directedReadOptionsForRequest, + boolean readOnly) { + if (!readOnly) { + if ((directedReadOptionsForRequest != null) || (directedReadOptionsForClient != null)) { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.FAILED_PRECONDITION, + "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); + } + } + // If DirectedReadOptions is not set at request-level, the request object won't be + // having DirectedReadOptions field set. Though, if DirectedReadOptions is set at client-level + // (through SpannerOptions), we must modify the request object to set the DirectedReadOptions + // proto field to this value. + if ((directedReadOptionsForRequest == null) && (directedReadOptionsForClient != null)) { + return directedReadOptionsForClient; + } + return directedReadOptionsForRequest; + } } 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 1db967fc02e..4bce749aba7 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 @@ -68,6 +68,7 @@ import com.google.cloud.spanner.SpannerOptions; import com.google.cloud.spanner.SpannerOptions.CallContextConfigurator; import com.google.cloud.spanner.SpannerOptions.CallCredentialsProvider; +import com.google.cloud.spanner.SpannerUtil; 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.GrpcDatabaseAdminCallableFactory; @@ -1643,7 +1644,14 @@ public StreamingCall read( ResultStreamConsumer consumer, @Nullable Map options, boolean readOnly) { - request = validateReadRequest(request, readOnly); + DirectedReadOptions preferredDirectedReadOptions = + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptions, + request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, + readOnly); + if (preferredDirectedReadOptions != null) { + request = request.toBuilder().setDirectedReadOptions(preferredDirectedReadOptions).build(); + } GrpcCallContext context = newCallContext(options, request.getSession(), request, SpannerGrpc.getReadMethod()); SpannerResponseObserver responseObserver = new SpannerResponseObserver(consumer); @@ -1673,7 +1681,14 @@ public ResultSet executeQuery( @Override public ApiFuture executeQueryAsync( ExecuteSqlRequest request, @Nullable Map options, boolean readOnly) { - request = validateExecuteSqlRequest(request, readOnly); + DirectedReadOptions preferredDirectedReadOptions = + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptions, + request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, + readOnly); + if (preferredDirectedReadOptions != null) { + request = request.toBuilder().setDirectedReadOptions(preferredDirectedReadOptions).build(); + } GrpcCallContext context = newCallContext(options, request.getSession(), request, SpannerGrpc.getExecuteSqlMethod()); return spannerStub.executeSqlCallable().futureCall(request, context); @@ -1682,7 +1697,10 @@ public ApiFuture executeQueryAsync( @Override public ResultSet executePartitionedDml( ExecuteSqlRequest request, @Nullable Map options) { - request = validateExecuteSqlRequest(request, false); + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptions, + request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, + false); GrpcCallContext context = newCallContext(options, request.getSession(), request, SpannerGrpc.getExecuteSqlMethod()); return get(partitionedDmlStub.executeSqlCallable().futureCall(request, context)); @@ -1696,7 +1714,10 @@ public RetrySettings getPartitionedDmlRetrySettings() { @Override public ServerStream executeStreamingPartitionedDml( ExecuteSqlRequest request, Map options, Duration timeout) { - request = validateExecuteSqlRequest(request, false); + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptions, + request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, + false); GrpcCallContext context = newCallContext( options, request.getSession(), request, SpannerGrpc.getExecuteStreamingSqlMethod()); @@ -1711,7 +1732,14 @@ public StreamingCall executeQuery( ResultStreamConsumer consumer, @Nullable Map options, boolean readOnly) { - request = validateExecuteSqlRequest(request, readOnly); + DirectedReadOptions preferredDirectedReadOptions = + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptions, + request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, + readOnly); + if (preferredDirectedReadOptions != null) { + request = request.toBuilder().setDirectedReadOptions(preferredDirectedReadOptions).build(); + } GrpcCallContext context = newCallContext( options, request.getSession(), request, SpannerGrpc.getExecuteStreamingSqlMethod()); @@ -2029,41 +2057,4 @@ private static Duration systemProperty(String name, int defaultValue) { String stringValue = System.getProperty(name, ""); return Duration.ofSeconds(stringValue.isEmpty() ? defaultValue : Integer.parseInt(stringValue)); } - - @VisibleForTesting - ExecuteSqlRequest validateExecuteSqlRequest(ExecuteSqlRequest request, boolean readOnly) { - if (!readOnly) { - if (request.hasDirectedReadOptions() || (directedReadOptions != null)) { - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.FAILED_PRECONDITION, - "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); - } - } - // If DirectedReadOptions is not set at request-level, the request object won't be - // having DirectedReadOptions field set. Though, if DirectedReadOptions is set at client-level - // (through SpannerOptions), we must modify the request object to set the DirectedReadOptions - // proto field to this value. - if (!request.hasDirectedReadOptions() && directedReadOptions != null) { - return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); - } - return request; - } - - ReadRequest validateReadRequest(ReadRequest request, boolean readOnly) { - if (!readOnly) { - if (request.hasDirectedReadOptions() || (directedReadOptions != null)) { - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.FAILED_PRECONDITION, - "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); - } - } - // If DirectedReadOptions is not set at request-level, the request object won't be - // having DirectedReadOptions field set. Though, if DirectedReadOptions is set at client-level - // (through SpannerOptions), we must modify the request object to set the DirectedReadOptions - // proto field to this value. - if (!request.hasDirectedReadOptions() && directedReadOptions != null) { - return request.toBuilder().setDirectedReadOptions(directedReadOptions).build(); - } - return request; - } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java index 4107306853b..401a3f69b53 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java @@ -80,4 +80,63 @@ public void testDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterT () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } + + @Test + public void testValidateDirectedReadOptions() { + DirectedReadOptions directedReadOptions = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build())) + .build(); + SpannerException e = + assertThrows( + SpannerException.class, + () -> { + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + null, directedReadOptions, false); + }); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + + e = + assertThrows( + SpannerException.class, + () -> { + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptions, null, false); + }); + assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); + } + + @Test + public void testGetPreferredDirectedReadOptions() { + DirectedReadOptions directedReadOptionsForClient = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build())) + .build(); + DirectedReadOptions directedReadOptionsForRequest = + DirectedReadOptions.newBuilder() + .setExcludeReplicas( + ExcludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-east1").build())) + .build(); + + assertEquals( + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptionsForClient, directedReadOptionsForRequest, true), + directedReadOptionsForRequest); + assertEquals( + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + directedReadOptionsForClient, null, true), + directedReadOptionsForClient); + assertEquals( + SpannerUtil.validateAndGetPreferredDirectedReadOptions( + null, directedReadOptionsForRequest, true), + directedReadOptionsForRequest); + } } 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 1c71da67b1f..88c54152e20 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 @@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assume.assumeTrue; @@ -52,7 +51,6 @@ import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.GetSessionRequest; -import com.google.spanner.v1.ReadRequest; import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.SpannerGrpc; import com.google.spanner.v1.StructType; @@ -481,126 +479,6 @@ public void testCustomUserAgent() { } } - @Test - public void testValidateExecuteSqlRequest() { - ExecuteSqlRequest.Builder requestBuilder = - ExecuteSqlRequest.newBuilder().setSql("SELECT * FROM FOO"); - ExecuteSqlRequest requestWithoutDirectedReadOptions = requestBuilder.build(); - ExecuteSqlRequest requestWithDirectedReadOptions = - requestBuilder.setDirectedReadOptions(DIRECTED_READ_OPTIONS2).build(); - - // Case 1: Read-only transaction. - // Case 1.1: DirectedReadOptions passed in via SpannerOptions only. - ExecuteSqlRequest returnedRequest = - RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithoutDirectedReadOptions, true); - assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); - // Case 1.2: DirectedReadOptions passed in via request only. - returnedRequest = - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithDirectedReadOptions, true); - assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and request. - returnedRequest = - RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithDirectedReadOptions, true); - assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and request. - returnedRequest = - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithoutDirectedReadOptions, true); - assertFalse(returnedRequest.hasDirectedReadOptions()); - - // Case 2: Read-write/PDML transaction. - // Case 2.1: DirectedReadOptions passed in via SpannerOptions only. - SpannerException e = - assertThrows( - SpannerException.class, - () -> - RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithoutDirectedReadOptions, false)); - assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.2: DirectedReadOptions passed in via request only. - e = - assertThrows( - SpannerException.class, - () -> - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithDirectedReadOptions, false)); - assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and request. - e = - assertThrows( - SpannerException.class, - () -> - RPC_WITH_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithDirectedReadOptions, false)); - assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and request. - returnedRequest = - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateExecuteSqlRequest( - requestWithoutDirectedReadOptions, false); - assertFalse(returnedRequest.hasDirectedReadOptions()); - } - - @Test - public void testValidateReadRequest() { - ReadRequest.Builder requestBuilder = ReadRequest.newBuilder().setTable("tbl"); - ReadRequest requestWithoutDirectedReadOptions = requestBuilder.build(); - ReadRequest requestWithDirectedReadOptions = - requestBuilder.setDirectedReadOptions(DIRECTED_READ_OPTIONS2).build(); - - // Case 1: Read-only transaction. - // Case 1.1: DirectedReadOptions passed in via SpannerOptions only. - ReadRequest returnedRequest = - RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithoutDirectedReadOptions, true); - assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS1); - // Case 1.2: DirectedReadOptions passed in via request only. - returnedRequest = - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithDirectedReadOptions, true); - assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.3: DirectedReadOptions passed in via both SpannerOptions and request. - returnedRequest = - RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest(requestWithDirectedReadOptions, true); - assertEquals(returnedRequest.getDirectedReadOptions(), DIRECTED_READ_OPTIONS2); - // Case 1.4: DirectedReadOptions passed in via neither SpannerOptions and request. - returnedRequest = - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( - requestWithoutDirectedReadOptions, true); - assertFalse(returnedRequest.hasDirectedReadOptions()); - - // Case 2: Read-write/PDML transaction. - // Case 2.1: DirectedReadOptions passed in via SpannerOptions only. - SpannerException e = - assertThrows( - SpannerException.class, - () -> - RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest( - requestWithoutDirectedReadOptions, false)); - assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.2: DirectedReadOptions passed in via request only. - e = - assertThrows( - SpannerException.class, - () -> - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( - requestWithDirectedReadOptions, false)); - assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.3: DirectedReadOptions passed in via both SpannerOptions and request. - e = - assertThrows( - SpannerException.class, - () -> - RPC_WITH_DIRECTED_READ_OPTIONS.validateReadRequest( - requestWithDirectedReadOptions, false)); - assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); - // Case 2.4: DirectedReadOptions passed in via both SpannerOptions and request. - returnedRequest = - RPC_WITHOUT_DIRECTED_READ_OPTIONS.validateReadRequest( - requestWithoutDirectedReadOptions, false); - assertFalse(returnedRequest.hasDirectedReadOptions()); - } - private SpannerOptions createSpannerOptions() { String endpoint = address.getHostString() + ":" + server.getPort(); return SpannerOptions.newBuilder() From 167659da66e94b64a713375d0794a907b6f16b3b Mon Sep 17 00:00:00 2001 From: rajatrb Date: Fri, 21 Apr 2023 10:03:57 +0000 Subject: [PATCH 11/18] move validation of directedreadoptions check to while creating Options or SpannerOptions. --- .../cloud/spanner/AbstractReadContext.java | 2 -- .../com/google/cloud/spanner/Options.java | 2 ++ .../com/google/cloud/spanner/OptionsTest.java | 7 ++++++ .../cloud/spanner/SpannerOptionsTest.java | 24 +++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index 8d05d86b471..a5bc774956a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -602,7 +602,6 @@ ExecuteSqlRequest.Builder getExecuteSqlRequestBuilder( builder.setDataBoostEnabled(options.dataBoostEnabled()); } if (options.hasDirectedReadOptions()) { - SpannerUtil.verifyDirectedReadOptions(options.directedReadOptions()); builder.setDirectedReadOptions(options.directedReadOptions()); } builder.setSeqno(getSeqNo()); @@ -788,7 +787,6 @@ ResultSet readInternalWithOptions( builder.setDataBoostEnabled(readOptions.dataBoostEnabled()); } if (readOptions.hasDirectedReadOptions()) { - SpannerUtil.verifyDirectedReadOptions(readOptions.directedReadOptions()); builder.setDirectedReadOptions(readOptions.directedReadOptions()); } final int prefetchChunks = diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index fbb79f84753..86065b8a8c8 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -338,6 +338,8 @@ static final class DirectedReadOption extends InternalOption implements ReadAndQ private final DirectedReadOptions directedReadOptions; DirectedReadOption(DirectedReadOptions directedReadOptions) { + Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); + SpannerUtil.verifyDirectedReadOptions(directedReadOptions); this.directedReadOptions = directedReadOptions; } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java index 4ebbc75be10..18f7c7fa4f7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java @@ -107,6 +107,13 @@ public void allOptionsAbsent() { assertEquals(options.hashCode(), 31); } + @Test + public void directedReadsNullNotAllowed() { + assertThrows( + NullPointerException.class, + () -> Options.directedRead(null)); + } + @Test public void listOptionsTest() { int pageSize = 3; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 55a15809a48..08bd77ba6c0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -44,6 +44,9 @@ import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.CreateSessionRequest; import com.google.spanner.v1.DeleteSessionRequest; +import com.google.spanner.v1.DirectedReadOptions; +import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; +import com.google.spanner.v1.DirectedReadOptions.ReplicaSelection; import com.google.spanner.v1.ExecuteBatchDmlRequest; import com.google.spanner.v1.ExecuteSqlRequest; import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; @@ -570,6 +573,27 @@ public void testSetEmulatorHostWithProtocol() { assertThat(options.getEndpoint()).isEqualTo("localhost:1234"); } + @Test + public void testSetDirectedReadOptions() { + final DirectedReadOptions directedReadOptions = + DirectedReadOptions.newBuilder() + .setIncludeReplicas( + IncludeReplicas.newBuilder() + .addReplicaSelections( + ReplicaSelection.newBuilder().setLocation("us-west1").build()) + .build()) + .build(); + SpannerOptions options = + SpannerOptions.newBuilder() + .setProjectId("[PROJECT]") + .setDirectedReadOptions(directedReadOptions) + .build(); + assertEquals(options.getDirectedReadOptions(), directedReadOptions); + assertThrows( + NullPointerException.class, + () -> SpannerOptions.newBuilder().setDirectedReadOptions(null).build()); + } + @Test public void testDefaultQueryOptions() { SpannerOptions.useEnvironment( From 0f876391d88560e8cd6550b3aa12c45bc2014922 Mon Sep 17 00:00:00 2001 From: rajatrb Date: Fri, 21 Apr 2023 10:08:10 +0000 Subject: [PATCH 12/18] revert changes to AsyncTransactionManagerImplTest --- .../google/cloud/spanner/AsyncTransactionManagerImplTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java index e6dd3a1a659..84f7fb0db9b 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerImplTest.java @@ -40,6 +40,7 @@ public void testCommitReturnsCommitStats() { new AsyncTransactionManagerImpl(session, mock(Span.class), Options.commitStats())) { when(session.newTransaction(Options.fromTransactionOptions(Options.commitStats()))) .thenReturn(transaction); + when(transaction.ensureTxnAsync()).thenReturn(ApiFutures.immediateFuture(null)); Timestamp commitTimestamp = Timestamp.ofTimeMicroseconds(1); CommitResponse response = mock(CommitResponse.class); when(response.getCommitTimestamp()).thenReturn(commitTimestamp); From f575fb4378f565b0d0efa1db4544bd077dce3050 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Fri, 25 Aug 2023 16:21:52 +0530 Subject: [PATCH 13/18] fix interfaces, lint --- .../google/cloud/spanner/AbstractReadContext.java | 12 ++++++++++-- .../com/google/cloud/spanner/SpannerOptions.java | 1 + .../google/cloud/spanner/TransactionRunnerImpl.java | 4 +++- .../google/cloud/spanner/spi/v1/GapicSpannerRpc.java | 2 +- .../com/google/cloud/spanner/spi/v1/SpannerRpc.java | 7 ++----- .../java/com/google/cloud/spanner/OptionsTest.java | 4 +--- .../com/google/cloud/spanner/SessionImplTest.java | 7 ++++++- .../com/google/cloud/spanner/SessionPoolTest.java | 3 ++- .../cloud/spanner/TransactionManagerImplTest.java | 5 ++++- 9 files changed, 30 insertions(+), 15 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index c18e4e01829..a0bc9f280af 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -694,7 +694,11 @@ CloseableIterator startStream(@Nullable ByteString resumeToken } SpannerRpc.StreamingCall call = rpc.executeQuery( - request.build(), stream.consumer(), session.getOptions(), isRouteToLeader(), readOnly); + request.build(), + stream.consumer(), + session.getOptions(), + isRouteToLeader(), + readOnly); call.request(prefetchChunks); stream.setCall(call, request.getTransaction().hasBegin()); return stream; @@ -834,7 +838,11 @@ CloseableIterator startStream(@Nullable ByteString resumeToken builder.setRequestOptions(buildRequestOptions(readOptions)); SpannerRpc.StreamingCall call = rpc.read( - builder.build(), stream.consumer(), session.getOptions(), isRouteToLeader(), readOnly); + builder.build(), + stream.consumer(), + session.getOptions(), + isRouteToLeader(), + readOnly); call.request(prefetchChunks); stream.setCall(call, /* withBeginTransaction = */ builder.getTransaction().hasBegin()); return stream; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 96af8551a04..03f467fd8af 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1347,6 +1347,7 @@ public String getCompressorName() { public DirectedReadOptions getDirectedReadOptions() { return directedReadOptions; } + public boolean isLeaderAwareRoutingEnabled() { return leaderAwareRoutingEnabled; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index 5b4434ed70c..57f8174a1d6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -753,7 +753,9 @@ public ApiFuture executeUpdateAsync(Statement statement, UpdateOption... o // Register the update as an async operation that must finish before the transaction may // commit. increaseAsyncOperations(); - resultSet = rpc.executeQueryAsync(builder.build(), session.getOptions(), isRouteToLeader(), readOnly); + resultSet = + rpc.executeQueryAsync( + builder.build(), session.getOptions(), isRouteToLeader(), readOnly); } catch (Throwable t) { decreaseAsyncOperations(); throw t; 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 ac57dbb6af3..9dd9e53d13b 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 @@ -1621,7 +1621,7 @@ public StreamingCall read( ReadRequest request, ResultStreamConsumer consumer, @Nullable Map options, - boolean routeToLeader + boolean routeToLeader, boolean readOnly) { DirectedReadOptions preferredDirectedReadOptions = SpannerUtil.validateAndGetPreferredDirectedReadOptions( 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 e2083f68979..71a8335d9d6 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 @@ -365,9 +365,6 @@ StreamingCall read( boolean routeToLeader, boolean readOnly); - ResultSet executeQuery( - ExecuteSqlRequest request, @Nullable Map options, boolean routeToLeader, boolean readOnly); - /** Returns the retry settings for streaming query operations. */ default RetrySettings getExecuteQueryRetrySettings() { return SpannerStubSettings.newBuilder().executeStreamingSqlSettings().getRetrySettings(); @@ -387,7 +384,7 @@ default Set getExecuteQueryRetryableCodes() { * PartitionRead/PartitionQuery) are preferred to be routed to any region for optimal latency. */ ResultSet executeQuery( - ExecuteSqlRequest request, @Nullable Map options, boolean routeToLeader); + ExecuteSqlRequest request, @Nullable Map options, boolean routeToLeader, boolean readOnly); /** * Executes a query asynchronously. @@ -419,7 +416,7 @@ StreamingCall executeQuery( ExecuteSqlRequest request, ResultStreamConsumer consumer, @Nullable Map options, - boolean routeToLeader + boolean routeToLeader, boolean readOnly); ExecuteBatchDmlResponse executeBatchDml(ExecuteBatchDmlRequest build, Map options); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java index 18f7c7fa4f7..d2174ab73e5 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OptionsTest.java @@ -109,9 +109,7 @@ public void allOptionsAbsent() { @Test public void directedReadsNullNotAllowed() { - assertThrows( - NullPointerException.class, - () -> Options.directedRead(null)); + assertThrows(NullPointerException.class, () -> Options.directedRead(null)); } @Test 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 ed165660cd6..35fb537e4f3 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 @@ -436,7 +436,12 @@ private void mockRead(final PartialResultSet myResultSet) { final ArgumentCaptor consumer = ArgumentCaptor.forClass(SpannerRpc.ResultStreamConsumer.class); Mockito.when( - rpc.read(Mockito.any(), consumer.capture(), Mockito.eq(options), eq(false), Mockito.anyBoolean())) + rpc.read( + Mockito.any(), + consumer.capture(), + Mockito.eq(options), + eq(false), + Mockito.anyBoolean())) .then( invocation -> { consumer.getValue().onPartialResultSet(myResultSet); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java index 0edaf51fad0..a9da5f98524 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java @@ -1125,7 +1125,8 @@ public void testSessionNotFoundReadWriteTransaction() { eq(true), any(Boolean.class))) .thenReturn(closedStreamingCall); - when(rpc.executeQuery(any(ExecuteSqlRequest.class), any(Map.class), eq(true), any(Boolean.class))) + when(rpc.executeQuery( + any(ExecuteSqlRequest.class), any(Map.class), eq(true), any(Boolean.class))) .thenThrow(sessionNotFound); when(rpc.executeBatchDml(any(ExecuteBatchDmlRequest.class), any(Map.class))) .thenThrow(sessionNotFound); 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 bae86f58afc..9d572202c98 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 @@ -341,7 +341,10 @@ public void inlineBegin() { // We should have 2 ExecuteSql requests. verify(rpc, times(2)) .executeQuery( - Mockito.any(ExecuteSqlRequest.class), Mockito.anyMap(), eq(true), Mockito.anyBoolean()); + Mockito.any(ExecuteSqlRequest.class), + Mockito.anyMap(), + eq(true), + Mockito.anyBoolean()); // But only 1 with a BeginTransaction. assertThat(transactionsStarted.get()).isEqualTo(1); } From 0bc3171a3f4800dc310a6d1cdb9317cb06603470 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 25 Aug 2023 10:54:35 +0000 Subject: [PATCH 14/18] =?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 --- .../google/cloud/spanner/spi/v1/GapicSpannerRpc.java | 10 ++++++++-- .../com/google/cloud/spanner/spi/v1/SpannerRpc.java | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) 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 9dd9e53d13b..10c8dbff3dd 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 @@ -1651,13 +1651,19 @@ public Set getExecuteQueryRetryableCodes() { @Override public ResultSet executeQuery( - ExecuteSqlRequest request, @Nullable Map options, boolean routeToLeader, boolean readOnly) { + ExecuteSqlRequest request, + @Nullable Map options, + boolean routeToLeader, + boolean readOnly) { return get(executeQueryAsync(request, options, routeToLeader, readOnly)); } @Override public ApiFuture executeQueryAsync( - ExecuteSqlRequest request, @Nullable Map options, boolean routeToLeader, boolean readOnly) { + ExecuteSqlRequest request, + @Nullable Map options, + boolean routeToLeader, + boolean readOnly) { DirectedReadOptions preferredDirectedReadOptions = SpannerUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptions, 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 71a8335d9d6..453c2160922 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 @@ -384,7 +384,10 @@ default Set getExecuteQueryRetryableCodes() { * PartitionRead/PartitionQuery) are preferred to be routed to any region for optimal latency. */ ResultSet executeQuery( - ExecuteSqlRequest request, @Nullable Map options, boolean routeToLeader, boolean readOnly); + ExecuteSqlRequest request, + @Nullable Map options, + boolean routeToLeader, + boolean readOnly); /** * Executes a query asynchronously. @@ -395,7 +398,10 @@ ResultSet executeQuery( * PartitionRead/PartitionQuery) are preferred to be routed to any region for optimal latency. */ ApiFuture executeQueryAsync( - ExecuteSqlRequest request, @Nullable Map options, boolean routeToLeader, boolean readOnly); + ExecuteSqlRequest request, + @Nullable Map options, + boolean routeToLeader, + boolean readOnly); ResultSet executePartitionedDml(ExecuteSqlRequest request, @Nullable Map options); From 2dc5fe2498b7b5ed5d68a46a506bba83ab406748 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Fri, 25 Aug 2023 16:46:09 +0530 Subject: [PATCH 15/18] refactor --- .../com/google/cloud/spanner/Options.java | 3 +- .../google/cloud/spanner/SpannerOptions.java | 10 ++----- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 12 ++++---- .../DirectedReadsUtil.java} | 10 +++++-- .../DirectedReadsUtilTest.java} | 29 ++++++++++--------- 5 files changed, 34 insertions(+), 30 deletions(-) rename google-cloud-spanner/src/main/java/com/google/cloud/spanner/{SpannerUtil.java => util/DirectedReadsUtil.java} (90%) rename google-cloud-spanner/src/test/java/com/google/cloud/spanner/{SpannerUtilTest.java => util/DirectedReadsUtilTest.java} (82%) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index fa3b5323bab..a02514b63c6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -16,6 +16,7 @@ package com.google.cloud.spanner; +import com.google.cloud.spanner.util.DirectedReadsUtil; import com.google.common.base.Preconditions; import com.google.spanner.v1.DirectedReadOptions; import com.google.spanner.v1.RequestOptions.Priority; @@ -336,7 +337,7 @@ static final class DirectedReadOption extends InternalOption implements ReadAndQ DirectedReadOption(DirectedReadOptions directedReadOptions) { Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); - SpannerUtil.verifyDirectedReadOptions(directedReadOptions); + DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions); this.directedReadOptions = directedReadOptions; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 03f467fd8af..aad7427af1a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -40,6 +40,7 @@ import com.google.cloud.spanner.spi.SpannerRpcFactory; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.cloud.spanner.util.DirectedReadsUtil; import com.google.cloud.spanner.v1.SpannerSettings; import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.common.annotations.VisibleForTesting; @@ -1101,17 +1102,12 @@ public Builder setCompressorName(@Nullable String compressorName) { * Sets the {@link DirectedReadOptions} that specify which replicas or regions should be used * for non-transactional reads or queries. * - *

Note that DirectedReadOptions can only be set for ReadOnlyTransaction and - * SingleUseTransaction; it cannot be set for ReadWriteTransaction and - * PartitionedDmlTransaction. - * *

DirectedReadOptions set at the request level will take precedence over the options set - * using this method. If DirectedReadOptions are set at the request level, they will override - * the options set here when the RPC call is made. + * using this method. */ public Builder setDirectedReadOptions(DirectedReadOptions directedReadOptions) { Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); - SpannerUtil.verifyDirectedReadOptions(directedReadOptions); + DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions); this.directedReadOptions = directedReadOptions; return this; } 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 10c8dbff3dd..341d5ac909b 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 @@ -68,7 +68,6 @@ import com.google.cloud.spanner.SpannerOptions; import com.google.cloud.spanner.SpannerOptions.CallContextConfigurator; import com.google.cloud.spanner.SpannerOptions.CallCredentialsProvider; -import com.google.cloud.spanner.SpannerUtil; 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.GrpcDatabaseAdminCallableFactory; @@ -77,6 +76,7 @@ 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.encryption.EncryptionConfigProtoMapper; +import com.google.cloud.spanner.util.DirectedReadsUtil; import com.google.cloud.spanner.v1.stub.GrpcSpannerStub; import com.google.cloud.spanner.v1.stub.SpannerStub; import com.google.cloud.spanner.v1.stub.SpannerStubSettings; @@ -1624,7 +1624,7 @@ public StreamingCall read( boolean routeToLeader, boolean readOnly) { DirectedReadOptions preferredDirectedReadOptions = - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptions, request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, readOnly); @@ -1665,7 +1665,7 @@ public ApiFuture executeQueryAsync( boolean routeToLeader, boolean readOnly) { DirectedReadOptions preferredDirectedReadOptions = - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptions, request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, readOnly); @@ -1685,7 +1685,7 @@ public ApiFuture executeQueryAsync( @Override public ResultSet executePartitionedDml( ExecuteSqlRequest request, @Nullable Map options) { - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptions, request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, false); @@ -1703,7 +1703,7 @@ public RetrySettings getPartitionedDmlRetrySettings() { @Override public ServerStream executeStreamingPartitionedDml( ExecuteSqlRequest request, Map options, Duration timeout) { - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptions, request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, false); @@ -1727,7 +1727,7 @@ public StreamingCall executeQuery( boolean routeToLeader, boolean readOnly) { DirectedReadOptions preferredDirectedReadOptions = - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptions, request.hasDirectedReadOptions() ? request.getDirectedReadOptions() : null, readOnly); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/util/DirectedReadsUtil.java similarity index 90% rename from google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java rename to google-cloud-spanner/src/main/java/com/google/cloud/spanner/util/DirectedReadsUtil.java index 52ef8308a88..9dae59282ad 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerUtil.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/util/DirectedReadsUtil.java @@ -14,12 +14,16 @@ * limitations under the License. */ -package com.google.cloud.spanner; +package com.google.cloud.spanner.util; +import com.google.api.core.InternalApi; +import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.spanner.v1.DirectedReadOptions; -/** Utility methods for Spanner. */ -public class SpannerUtil { +/** Utility methods for DirectedReads feature. */ +@InternalApi +public class DirectedReadsUtil { static final int MAX_REPLICA_SELECTIONS_COUNT = 10; public static void verifyDirectedReadOptions(DirectedReadOptions directedReadOptions) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/util/DirectedReadsUtilTest.java similarity index 82% rename from google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java rename to google-cloud-spanner/src/test/java/com/google/cloud/spanner/util/DirectedReadsUtilTest.java index 401a3f69b53..7b89a563708 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerUtilTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/util/DirectedReadsUtilTest.java @@ -14,11 +14,13 @@ * limitations under the License. */ -package com.google.cloud.spanner; +package com.google.cloud.spanner.util; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.SpannerException; import com.google.spanner.v1.DirectedReadOptions; import com.google.spanner.v1.DirectedReadOptions.ExcludeReplicas; import com.google.spanner.v1.DirectedReadOptions.IncludeReplicas; @@ -26,19 +28,20 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Unit tests for {@link com.google.cloud.spanner.SpannerUtil}. */ +/** Unit tests for {@link com.google.cloud.spanner.util.DirectedReadsUtil}. */ @RunWith(JUnit4.class) -public class SpannerUtilTest { +public class DirectedReadsUtilTest { private DirectedReadOptions getDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterThanMax() { List replicaSelectionList = new ArrayList<>( Collections.nCopies( - SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, + DirectedReadsUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, ReplicaSelection.newBuilder().setLocation("us-west1").build())); return DirectedReadOptions.newBuilder() .setIncludeReplicas( @@ -51,7 +54,7 @@ public class SpannerUtilTest { List replicaSelectionList = new ArrayList<>( Collections.nCopies( - SpannerUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, + DirectedReadsUtil.MAX_REPLICA_SELECTIONS_COUNT + 1, ReplicaSelection.newBuilder().setLocation("us-east1").build())); return DirectedReadOptions.newBuilder() .setExcludeReplicas( @@ -66,8 +69,8 @@ public void testDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterT SpannerException e = assertThrows( SpannerException.class, - () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); - assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); + () -> DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions)); + Assert.assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } @Test @@ -77,7 +80,7 @@ public void testDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterT SpannerException e = assertThrows( SpannerException.class, - () -> SpannerUtil.verifyDirectedReadOptions(directedReadOptions)); + () -> DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions)); assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } @@ -94,7 +97,7 @@ public void testValidateDirectedReadOptions() { assertThrows( SpannerException.class, () -> { - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( null, directedReadOptions, false); }); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); @@ -103,7 +106,7 @@ public void testValidateDirectedReadOptions() { assertThrows( SpannerException.class, () -> { - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptions, null, false); }); assertEquals(e.getErrorCode(), ErrorCode.FAILED_PRECONDITION); @@ -127,15 +130,15 @@ public void testGetPreferredDirectedReadOptions() { .build(); assertEquals( - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptionsForClient, directedReadOptionsForRequest, true), directedReadOptionsForRequest); assertEquals( - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( directedReadOptionsForClient, null, true), directedReadOptionsForClient); assertEquals( - SpannerUtil.validateAndGetPreferredDirectedReadOptions( + DirectedReadsUtil.validateAndGetPreferredDirectedReadOptions( null, directedReadOptionsForRequest, true), directedReadOptionsForRequest); } From b35392577cac2999d8bbd2b42e30cef73b252873 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Sat, 26 Aug 2023 02:23:19 +0530 Subject: [PATCH 16/18] address review comments --- .../main/java/com/google/cloud/spanner/Options.java | 8 +++++--- .../com/google/cloud/spanner/SpannerOptions.java | 7 ++++--- .../google/cloud/spanner/util/DirectedReadsUtil.java | 12 +++++++----- .../cloud/spanner/util/DirectedReadsUtilTest.java | 4 ++-- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index a02514b63c6..a17b842acac 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -336,9 +336,11 @@ static final class DirectedReadOption extends InternalOption implements ReadAndQ private final DirectedReadOptions directedReadOptions; DirectedReadOption(DirectedReadOptions directedReadOptions) { - Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); - DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions); - this.directedReadOptions = directedReadOptions; + this.directedReadOptions = + Preconditions.checkNotNull( + DirectedReadsUtil.validateDirectedReadOptions(directedReadOptions), + "DirectedReadOptions cannot be null"); + ; } @Override diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index aad7427af1a..59ca7e7ac4a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1106,9 +1106,10 @@ public Builder setCompressorName(@Nullable String compressorName) { * using this method. */ public Builder setDirectedReadOptions(DirectedReadOptions directedReadOptions) { - Preconditions.checkNotNull(directedReadOptions, "DirectedReadOptions cannot be null"); - DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions); - this.directedReadOptions = directedReadOptions; + this.directedReadOptions = + Preconditions.checkNotNull( + DirectedReadsUtil.validateDirectedReadOptions(directedReadOptions), + "DirectedReadOptions cannot be null"); return this; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/util/DirectedReadsUtil.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/util/DirectedReadsUtil.java index 9dae59282ad..a39f6762862 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/util/DirectedReadsUtil.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/util/DirectedReadsUtil.java @@ -26,7 +26,8 @@ public class DirectedReadsUtil { static final int MAX_REPLICA_SELECTIONS_COUNT = 10; - public static void verifyDirectedReadOptions(DirectedReadOptions directedReadOptions) { + public static DirectedReadOptions validateDirectedReadOptions( + DirectedReadOptions directedReadOptions) { if (directedReadOptions.hasIncludeReplicas() && directedReadOptions.hasExcludeReplicas()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INVALID_ARGUMENT, @@ -44,6 +45,7 @@ public static void verifyDirectedReadOptions(DirectedReadOptions directedReadOpt "Maximum length of replica selection allowed in IncludeReplicas/ExcludeReplicas is %d", MAX_REPLICA_SELECTIONS_COUNT)); } + return directedReadOptions; } public static DirectedReadOptions validateAndGetPreferredDirectedReadOptions( @@ -51,7 +53,7 @@ public static DirectedReadOptions validateAndGetPreferredDirectedReadOptions( DirectedReadOptions directedReadOptionsForRequest, boolean readOnly) { if (!readOnly) { - if ((directedReadOptionsForRequest != null) || (directedReadOptionsForClient != null)) { + if (directedReadOptionsForRequest != null || directedReadOptionsForClient != null) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.FAILED_PRECONDITION, "DirectedReadOptions can't be set for Read-Write or Partitioned DML transactions"); @@ -61,9 +63,9 @@ public static DirectedReadOptions validateAndGetPreferredDirectedReadOptions( // having DirectedReadOptions field set. Though, if DirectedReadOptions is set at client-level // (through SpannerOptions), we must modify the request object to set the DirectedReadOptions // proto field to this value. - if ((directedReadOptionsForRequest == null) && (directedReadOptionsForClient != null)) { - return directedReadOptionsForClient; + if (directedReadOptionsForRequest != null) { + return directedReadOptionsForRequest; } - return directedReadOptionsForRequest; + return directedReadOptionsForClient; } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/util/DirectedReadsUtilTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/util/DirectedReadsUtilTest.java index 7b89a563708..3f8b80c75ee 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/util/DirectedReadsUtilTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/util/DirectedReadsUtilTest.java @@ -69,7 +69,7 @@ public void testDirectedReadOptions_IncludeReplica_ReplicaSelectionCountGreaterT SpannerException e = assertThrows( SpannerException.class, - () -> DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions)); + () -> DirectedReadsUtil.validateDirectedReadOptions(directedReadOptions)); Assert.assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } @@ -80,7 +80,7 @@ public void testDirectedReadOptions_ExcludeReplica_ReplicaSelectionCountGreaterT SpannerException e = assertThrows( SpannerException.class, - () -> DirectedReadsUtil.verifyDirectedReadOptions(directedReadOptions)); + () -> DirectedReadsUtil.validateDirectedReadOptions(directedReadOptions)); assertEquals(e.getErrorCode(), ErrorCode.INVALID_ARGUMENT); } From e4ba3402bc6930f8a0e9c4d8a00c65ed28c47a7d Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Sat, 26 Aug 2023 02:29:34 +0530 Subject: [PATCH 17/18] address review comments --- .../src/main/java/com/google/cloud/spanner/Options.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index a17b842acac..39fc05ee167 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -226,7 +226,14 @@ public static CreateUpdateDeleteAdminApiOption validateOnly(Boolean validateOnly return new ValidateOnlyOption(validateOnly); } - /** Option to request DirectedRead for ReadOnlyTransaction and SingleUseTransaction. */ + /** + * Option to request DirectedRead for ReadOnlyTransaction and SingleUseTransaction. + *

+ * The DirectedReadOptions can be used to indicate which replicas or regions should be used for + * non-transactional reads or queries. Not all requests can be sent to non-leader replicas. In + * particular, some requests such as reads within read-write transactions must be sent to a + * designated leader replica. These requests ignore DirectedReadOptions. + */ public static ReadAndQueryOption directedRead(DirectedReadOptions directedReadOptions) { return new DirectedReadOption(directedReadOptions); } From 5692104f0ca53004c7f068e788e58abfcfb9ee85 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 25 Aug 2023 21:02:18 +0000 Subject: [PATCH 18/18] =?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 --- .../src/main/java/com/google/cloud/spanner/Options.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index 39fc05ee167..42e08993ef3 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -228,8 +228,8 @@ public static CreateUpdateDeleteAdminApiOption validateOnly(Boolean validateOnly /** * Option to request DirectedRead for ReadOnlyTransaction and SingleUseTransaction. - *

- * The DirectedReadOptions can be used to indicate which replicas or regions should be used for + * + *

The DirectedReadOptions can be used to indicate which replicas or regions should be used for * non-transactional reads or queries. Not all requests can be sent to non-leader replicas. In * particular, some requests such as reads within read-write transactions must be sent to a * designated leader replica. These requests ignore DirectedReadOptions.