Skip to content

Commit 1c9c48c

Browse files
olavloitekolea2
olavloite
authored and
kolea2
committed
fix retry test cases for database and instance admin operations (#6065)
Database and Instance admin operations have been changed from idempotent to non-idempotent. The corresponding test cases have been updated accordingly.
1 parent 16435cb commit 1c9c48c

File tree

2 files changed

+129
-46
lines changed

2 files changed

+129
-46
lines changed

‎google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java‎

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package com.google.cloud.spanner;
1818

19+
import static org.junit.Assert.fail;
20+
1921
import com.google.api.core.ApiFunction;
2022
import com.google.api.gax.grpc.testing.LocalChannelProvider;
2123
import com.google.api.gax.longrunning.OperationFuture;
@@ -243,28 +245,43 @@ public Void apply(Builder<?, ?> input) {
243245
return null;
244246
}
245247
});
246-
builder
248+
249+
if (!builder
247250
.getDatabaseAdminStubSettingsBuilder()
248251
.createDatabaseOperationSettings()
249-
.setInitialCallSettings(
250-
builder
251-
.getDatabaseAdminStubSettingsBuilder()
252-
.createDatabaseOperationSettings()
253-
.getInitialCallSettings()
254-
.toBuilder()
255-
.setRetrySettings(retrySettings)
256-
.build());
257-
builder
252+
.getInitialCallSettings()
253+
.getRetryableCodes()
254+
.isEmpty()) {
255+
builder
256+
.getDatabaseAdminStubSettingsBuilder()
257+
.createDatabaseOperationSettings()
258+
.setInitialCallSettings(
259+
builder
260+
.getDatabaseAdminStubSettingsBuilder()
261+
.createDatabaseOperationSettings()
262+
.getInitialCallSettings()
263+
.toBuilder()
264+
.setRetrySettings(retrySettings)
265+
.build());
266+
}
267+
if (!builder
258268
.getDatabaseAdminStubSettingsBuilder()
259269
.updateDatabaseDdlOperationSettings()
260-
.setInitialCallSettings(
261-
builder
262-
.getDatabaseAdminStubSettingsBuilder()
263-
.updateDatabaseDdlOperationSettings()
264-
.getInitialCallSettings()
265-
.toBuilder()
266-
.setRetrySettings(retrySettings)
267-
.build());
270+
.getInitialCallSettings()
271+
.getRetryableCodes()
272+
.isEmpty()) {
273+
builder
274+
.getDatabaseAdminStubSettingsBuilder()
275+
.updateDatabaseDdlOperationSettings()
276+
.setInitialCallSettings(
277+
builder
278+
.getDatabaseAdminStubSettingsBuilder()
279+
.updateDatabaseDdlOperationSettings()
280+
.getInitialCallSettings()
281+
.toBuilder()
282+
.setRetrySettings(retrySettings)
283+
.build());
284+
}
268285
spanner = builder.build().getService();
269286
client = spanner.getDatabaseAdminClient();
270287
}
@@ -369,19 +386,36 @@ public void createDatabaseTest() throws Exception {
369386
}
370387
mockDatabaseAdmin.addResponse(resultOperation);
371388

389+
boolean methodIsIdempotent =
390+
!spanner
391+
.getOptions()
392+
.getDatabaseAdminStubSettings()
393+
.createDatabaseOperationSettings()
394+
.getInitialCallSettings()
395+
.getRetryableCodes()
396+
.isEmpty();
372397
for (int i = 0; i < 2; i++) {
373398
OperationFuture<Database, CreateDatabaseMetadata> actualResponse =
374399
client.createDatabase(INSTANCE, "DATABASE", Arrays.<String>asList());
375400
try {
376401
Database returnedInstance = actualResponse.get();
402+
if (!methodIsIdempotent && i == exceptionAtCall) {
403+
fail("missing expected exception");
404+
}
377405
Assert.assertEquals(name.toString(), returnedInstance.getId().getName());
378406
} catch (ExecutionException e) {
379-
Throwables.throwIfUnchecked(e.getCause());
380-
throw e;
407+
if (!exceptionType.isRetryable() || methodIsIdempotent || i != exceptionAtCall) {
408+
Throwables.throwIfUnchecked(e.getCause());
409+
throw e;
410+
}
381411
}
382412
}
383413
List<AbstractMessage> actualRequests = mockDatabaseAdmin.getRequests();
384-
Assert.assertEquals(2, actualRequests.size());
414+
if (methodIsIdempotent) {
415+
Assert.assertEquals(2, actualRequests.size());
416+
} else {
417+
Assert.assertEquals(1, actualRequests.size());
418+
}
385419
}
386420

387421
@Test

‎google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java‎

Lines changed: 74 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package com.google.cloud.spanner;
1818

19+
import static org.junit.Assert.fail;
20+
1921
import com.google.api.core.ApiFunction;
2022
import com.google.api.gax.grpc.testing.LocalChannelProvider;
2123
import com.google.api.gax.longrunning.OperationFuture;
@@ -246,28 +248,42 @@ public Void apply(Builder<?, ?> input) {
246248
return null;
247249
}
248250
});
249-
builder
251+
if (!builder
250252
.getInstanceAdminStubSettingsBuilder()
251253
.createInstanceOperationSettings()
252-
.setInitialCallSettings(
253-
builder
254-
.getInstanceAdminStubSettingsBuilder()
255-
.createInstanceOperationSettings()
256-
.getInitialCallSettings()
257-
.toBuilder()
258-
.setRetrySettings(retrySettings)
259-
.build());
260-
builder
254+
.getInitialCallSettings()
255+
.getRetryableCodes()
256+
.isEmpty()) {
257+
builder
258+
.getInstanceAdminStubSettingsBuilder()
259+
.createInstanceOperationSettings()
260+
.setInitialCallSettings(
261+
builder
262+
.getInstanceAdminStubSettingsBuilder()
263+
.createInstanceOperationSettings()
264+
.getInitialCallSettings()
265+
.toBuilder()
266+
.setRetrySettings(retrySettings)
267+
.build());
268+
}
269+
if (!builder
261270
.getInstanceAdminStubSettingsBuilder()
262271
.updateInstanceOperationSettings()
263-
.setInitialCallSettings(
264-
builder
265-
.getInstanceAdminStubSettingsBuilder()
266-
.updateInstanceOperationSettings()
267-
.getInitialCallSettings()
268-
.toBuilder()
269-
.setRetrySettings(retrySettings)
270-
.build());
272+
.getInitialCallSettings()
273+
.getRetryableCodes()
274+
.isEmpty()) {
275+
builder
276+
.getInstanceAdminStubSettingsBuilder()
277+
.updateInstanceOperationSettings()
278+
.setInitialCallSettings(
279+
builder
280+
.getInstanceAdminStubSettingsBuilder()
281+
.updateInstanceOperationSettings()
282+
.getInitialCallSettings()
283+
.toBuilder()
284+
.setRetrySettings(retrySettings)
285+
.build());
286+
}
271287
spanner = builder.build().getService();
272288
client = spanner.getInstanceAdminClient();
273289
}
@@ -453,6 +469,14 @@ public void createInstanceTest() throws Exception {
453469
}
454470
mockInstanceAdmin.addResponse(resultOperation);
455471

472+
boolean methodIsIdempotent =
473+
!spanner
474+
.getOptions()
475+
.getInstanceAdminStubSettings()
476+
.createInstanceOperationSettings()
477+
.getInitialCallSettings()
478+
.getRetryableCodes()
479+
.isEmpty();
456480
for (int i = 0; i < 2; i++) {
457481
OperationFuture<Instance, CreateInstanceMetadata> actualResponse =
458482
client.createInstance(
@@ -462,14 +486,23 @@ public void createInstanceTest() throws Exception {
462486
.build());
463487
try {
464488
Instance returnedInstance = actualResponse.get();
489+
if (!methodIsIdempotent && i == exceptionAtCall) {
490+
fail("missing expected exception");
491+
}
465492
Assert.assertEquals(displayName, returnedInstance.getDisplayName());
466493
} catch (ExecutionException e) {
467-
Throwables.throwIfUnchecked(e.getCause());
468-
throw e;
494+
if (!exceptionType.isRetryable() || methodIsIdempotent || i != exceptionAtCall) {
495+
Throwables.throwIfUnchecked(e.getCause());
496+
throw e;
497+
}
469498
}
470499
}
471500
List<AbstractMessage> actualRequests = mockInstanceAdmin.getRequests();
472-
Assert.assertEquals(2, actualRequests.size());
501+
if (methodIsIdempotent) {
502+
Assert.assertEquals(2, actualRequests.size());
503+
} else {
504+
Assert.assertEquals(1, actualRequests.size());
505+
}
473506
}
474507

475508
@Test
@@ -501,6 +534,14 @@ public void updateInstanceTest() throws Exception {
501534
}
502535
mockInstanceAdmin.addResponse(resultOperation);
503536

537+
boolean methodIsIdempotent =
538+
!spanner
539+
.getOptions()
540+
.getInstanceAdminStubSettings()
541+
.updateInstanceOperationSettings()
542+
.getInitialCallSettings()
543+
.getRetryableCodes()
544+
.isEmpty();
504545
for (int i = 0; i < 2; i++) {
505546
OperationFuture<Instance, UpdateInstanceMetadata> actualResponse =
506547
client.updateInstance(
@@ -510,15 +551,23 @@ public void updateInstanceTest() throws Exception {
510551
.build());
511552
try {
512553
Instance returnedInstance = actualResponse.get();
554+
if (!methodIsIdempotent && i == exceptionAtCall) {
555+
fail("missing expected exception");
556+
}
513557
Assert.assertEquals(displayName, returnedInstance.getDisplayName());
514558
} catch (ExecutionException e) {
515-
Throwables.throwIfUnchecked(e.getCause());
516-
throw e;
559+
if (!exceptionType.isRetryable() || methodIsIdempotent || i != exceptionAtCall) {
560+
Throwables.throwIfUnchecked(e.getCause());
561+
throw e;
562+
}
517563
}
518564
}
519-
520565
List<AbstractMessage> actualRequests = mockInstanceAdmin.getRequests();
521-
Assert.assertEquals(2, actualRequests.size());
566+
if (methodIsIdempotent) {
567+
Assert.assertEquals(2, actualRequests.size());
568+
} else {
569+
Assert.assertEquals(1, actualRequests.size());
570+
}
522571
}
523572

524573
@Test

0 commit comments

Comments
 (0)