Skip to content
This repository was archived by the owner on Apr 7, 2026. It is now read-only.

Commit 7d45ace

Browse files
olavloiteolavloite
committed
fix: address review comments
1 parent 1f33c42 commit 7d45ace

10 files changed

Lines changed: 24 additions & 35 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ interface AsyncWork<R> {
5858
ApiFuture<Timestamp> getCommitTimestamp();
5959

6060
/**
61-
* Returns the {@link CommitResponse} of this transaction. {@link ApiFuture#get()} will throw an
61+
* Returns the {@link CommitResponse} of this transaction. {@link ApiFuture#get()} throws an
6262
* {@link ExecutionException} if the transaction did not commit.
6363
*/
6464
ApiFuture<CommitResponse> getCommitResponse();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,11 @@ public ApiFuture<Timestamp> commitAsync() {
133133
SpannerExceptionFactory.newSpannerException(
134134
ErrorCode.ABORTED, "Transaction already aborted"));
135135
}
136-
ApiFuture<CommitResponse> res = txn.commitAsync();
136+
ApiFuture<CommitResponse> commitResponseFuture = txn.commitAsync();
137137
txnState = TransactionState.COMMITTED;
138138

139139
ApiFutures.addCallback(
140-
res,
140+
commitResponseFuture,
141141
new ApiFutureCallback<CommitResponse>() {
142142
@Override
143143
public void onFailure(Throwable t) {
@@ -156,7 +156,7 @@ public void onSuccess(CommitResponse result) {
156156
},
157157
MoreExecutors.directExecutor());
158158
return ApiFutures.transform(
159-
res,
159+
commitResponseFuture,
160160
new ApiFunction<CommitResponse, Timestamp>() {
161161
@Override
162162
public Timestamp apply(CommitResponse input) {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ static CommitStats fromProto(com.google.spanner.v1.CommitResponse.CommitStats pr
4242
* row from a parent table that has the ON DELETE CASCADE annotation is also counted as one
4343
* mutation regardless of the number of interleaved child rows present. The exception to this is
4444
* if there are secondary indexes defined on rows being deleted, then the changes to the secondary
45-
* indexes will be counted individually. For example, if a table has 2 secondary indexes, deleting
46-
* a range of rows in the table will count as 1 mutation for the table, plus 2 mutations for each
47-
* row that is deleted because the rows in the secondary index might be scattered over the
48-
* key-space, making it impossible for Cloud Spanner to call a single delete range operation on
49-
* the secondary indexes. Secondary indexes include the foreign keys backing indexes.
45+
* indexes are counted individually. For example, if a table has 2 secondary indexes, deleting a
46+
* range of rows in the table counts as 1 mutation for the table, plus 2 mutations for each row
47+
* that is deleted because the rows in the secondary index might be scattered over the key-space,
48+
* making it impossible for Cloud Spanner to call a single delete range operation on the secondary
49+
* indexes. Secondary indexes include the foreign keys backing indexes.
5050
*/
5151
public long getMutationCount() {
5252
return mutationCount;

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

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,42 +19,35 @@
1919
import static org.mockito.Mockito.mock;
2020
import static org.mockito.Mockito.verify;
2121
import static org.mockito.Mockito.when;
22-
import static org.mockito.MockitoAnnotations.initMocks;
2322

2423
import com.google.api.core.ApiFutures;
2524
import com.google.cloud.Timestamp;
2625
import io.opencensus.trace.Span;
27-
import org.junit.Before;
2826
import org.junit.Test;
2927
import org.junit.runner.RunWith;
30-
import org.junit.runners.JUnit4;
3128
import org.mockito.Mock;
29+
import org.mockito.runners.MockitoJUnitRunner;
3230

33-
@RunWith(JUnit4.class)
31+
@RunWith(MockitoJUnitRunner.class)
3432
public class AsyncTransactionManagerImplTest {
3533

3634
@Mock private SessionImpl session;
37-
@Mock TransactionRunnerImpl.TransactionContextImpl txn;
38-
39-
@Before
40-
public void setUp() {
41-
initMocks(this);
42-
}
35+
@Mock TransactionRunnerImpl.TransactionContextImpl transaction;
4336

4437
@Test
4538
public void testCommitReturnsCommitStats() {
4639
try (AsyncTransactionManagerImpl manager =
4740
new AsyncTransactionManagerImpl(session, mock(Span.class), Options.commitStats())) {
4841
when(session.newTransaction(Options.fromTransactionOptions(Options.commitStats())))
49-
.thenReturn(txn);
50-
when(txn.ensureTxnAsync()).thenReturn(ApiFutures.<Void>immediateFuture(null));
42+
.thenReturn(transaction);
43+
when(transaction.ensureTxnAsync()).thenReturn(ApiFutures.<Void>immediateFuture(null));
5144
Timestamp commitTimestamp = Timestamp.ofTimeMicroseconds(1);
5245
CommitResponse response = mock(CommitResponse.class);
5346
when(response.getCommitTimestamp()).thenReturn(commitTimestamp);
54-
when(txn.commitAsync()).thenReturn(ApiFutures.immediateFuture(response));
47+
when(transaction.commitAsync()).thenReturn(ApiFutures.immediateFuture(response));
5548
manager.beginAsync();
5649
manager.commitAsync();
57-
verify(txn).commitAsync();
50+
verify(transaction).commitAsync();
5851
}
5952
}
6053
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package com.google.cloud.spanner;
1818

1919
import static org.junit.Assert.assertEquals;
20-
import static org.junit.Assert.assertFalse;
2120
import static org.junit.Assert.assertNotEquals;
2221

2322
import com.google.cloud.Timestamp;
@@ -77,8 +76,8 @@ public void testEqualsAndHashCode() {
7776
assertEquals(response3, response1);
7877
assertNotEquals(response2, response1);
7978
assertNotEquals(response3, response2);
80-
assertFalse(response1.equals(null));
81-
assertFalse(response1.equals(new Object()));
79+
assertNotEquals(response1, null);
80+
assertNotEquals(response1, new Object());
8281

8382
assertEquals(response3.hashCode(), response1.hashCode());
8483
assertNotEquals(response2.hashCode(), response1.hashCode());

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1489,7 +1489,7 @@ public Void run(TransactionContext transaction) throws Exception {
14891489
} catch (Throwable t) {
14901490
mockSpanner.unfreeze();
14911491
// Wait until the update actually finishes so it has returned a transaction.
1492-
// That ensures that the retry does not issue a BeginTransaction RPC.
1492+
// This ensures that the retry does not issue a BeginTransaction RPC.
14931493
SpannerApiFutures.get(updateCount);
14941494
throw t;
14951495
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,6 @@
187187
* }</pre>
188188
*/
189189
public class MockSpannerServiceImpl extends SpannerImplBase implements MockGrpcService {
190-
191190
private static class PartialResultSetsIterator implements Iterator<PartialResultSet> {
192191
private static final int MAX_ROWS_IN_CHUNK = 1;
193192

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,8 @@
1818

1919
import static com.google.common.truth.Truth.assertThat;
2020
import static org.junit.Assert.assertEquals;
21-
import static org.junit.Assert.assertFalse;
2221
import static org.junit.Assert.assertNotEquals;
2322
import static org.junit.Assert.assertNotNull;
24-
import static org.junit.Assert.assertTrue;
2523
import static org.junit.Assert.fail;
2624

2725
import org.junit.Test;
@@ -240,21 +238,21 @@ public void testFromTransactionOptions_toStringWithCommitStats() {
240238
public void testTransactionOptions_noOptionsAreEqual() {
241239
Options option1 = Options.fromTransactionOptions();
242240
Options option2 = Options.fromTransactionOptions();
243-
assertTrue(option1.equals(option2));
241+
assertEquals(option1, option2);
244242
}
245243

246244
@Test
247245
public void testTransactionOptions_withCommitStatsAreEqual() {
248246
Options option1 = Options.fromTransactionOptions(Options.commitStats());
249247
Options option2 = Options.fromTransactionOptions(Options.commitStats());
250-
assertTrue(option1.equals(option2));
248+
assertEquals(option1, option2);
251249
}
252250

253251
@Test
254252
public void testTransactionOptions_withCommitStatsAndOtherOptionAreNotEqual() {
255253
Options option1 = Options.fromTransactionOptions(Options.commitStats());
256254
Options option2 = Options.fromQueryOptions(Options.prefetchChunks(10));
257-
assertFalse(option1.equals(option2));
255+
assertNotEquals(option1, option2);
258256
}
259257

260258
@Test

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITAsyncAPITest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ public ApiFuture<Void> apply(TransactionContext transaction, Void input)
373373
assertEquals(4L, get(manager.getCommitResponse()).getCommitStats().getMutationCount());
374374
break;
375375
} catch (AbortedException e) {
376-
Thread.sleep(e.getRetryDelayInMillis() / 1000);
376+
Thread.sleep(e.getRetryDelayInMillis());
377377
context = manager.resetForRetryAsync();
378378
}
379379
}

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITTransactionManagerTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ public void testTransactionManagerReturnsCommitStats() throws InterruptedExcepti
235235
assertEquals(2L, manager.getCommitResponse().getCommitStats().getMutationCount());
236236
break;
237237
} catch (AbortedException e) {
238-
Thread.sleep(e.getRetryDelayInMillis() / 1000);
238+
Thread.sleep(e.getRetryDelayInMillis());
239239
transaction = manager.resetForRetry();
240240
}
241241
}

0 commit comments

Comments
 (0)