Skip to content

Commit a5e83e5

Browse files
mziccardmziccard
committed
Make DNS batch get zone/changerequest return null for NOT_FOUND
1 parent e213176 commit a5e83e5

File tree

4 files changed

+92
-16
lines changed

4 files changed

+92
-16
lines changed

‎gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,21 @@ public BaseServiceException(IOException exception, boolean idempotent) {
135135
this.debugInfo = debugInfo;
136136
}
137137

138-
public BaseServiceException(GoogleJsonError error, boolean idempotent) {
139-
this(error.getCode(), error.getMessage(), reason(error), idempotent);
138+
public BaseServiceException(GoogleJsonError googleJsonError, boolean idempotent) {
139+
super(googleJsonError.getMessage());
140+
Error error = new Error(googleJsonError.getCode(), reason(googleJsonError));
141+
this.code = error.code;
142+
this.reason = error.reason;
143+
this.retryable = isRetryable(idempotent, error);
144+
if (this.reason != null) {
145+
GoogleJsonError.ErrorInfo errorInfo = googleJsonError.getErrors().get(0);
146+
this.location = errorInfo.getLocation();
147+
this.debugInfo = (String) errorInfo.get("debugInfo");
148+
} else {
149+
this.location = null;
150+
this.debugInfo = null;
151+
}
152+
this.idempotent = idempotent;
140153
}
141154

142155
public BaseServiceException(int code, String message, String reason, boolean idempotent) {

‎gcloud-java-dns/src/main/java/com/google/cloud/dns/DnsBatch.java‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public DnsBatchResult<Page<Zone>> listZones(Dns.ZoneListOption... options) {
9191
public DnsBatchResult<Zone> createZone(ZoneInfo zone, Dns.ZoneOption... options) {
9292
DnsBatchResult<Zone> result = new DnsBatchResult<>();
9393
// todo this can cause misleading report of a failure, intended to be fixed within #924
94-
RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, true);
94+
RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, false, true);
9595
Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
9696
batch.addCreateZone(zone.toPb(), callback, optionMap);
9797
return result;
@@ -118,7 +118,7 @@ public DnsBatchResult<Boolean> deleteZone(String zoneName) {
118118
*/
119119
public DnsBatchResult<Zone> getZone(String zoneName, Dns.ZoneOption... options) {
120120
DnsBatchResult<Zone> result = new DnsBatchResult<>();
121-
RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, true);
121+
RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, true, true);
122122
Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
123123
batch.addGetZone(zoneName, callback, optionMap);
124124
return result;
@@ -186,7 +186,7 @@ public DnsBatchResult<Page<ChangeRequest>> listChangeRequests(String zoneName,
186186
public DnsBatchResult<ChangeRequest> getChangeRequest(String zoneName, String changeRequestId,
187187
Dns.ChangeRequestOption... options) {
188188
DnsBatchResult<ChangeRequest> result = new DnsBatchResult<>();
189-
RpcBatch.Callback<Change> callback = createChangeRequestCallback(zoneName, result, true);
189+
RpcBatch.Callback<Change> callback = createChangeRequestCallback(zoneName, result, true, true);
190190
Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
191191
batch.addGetChangeRequest(zoneName, changeRequestId, callback, optionMap);
192192
return result;
@@ -203,14 +203,15 @@ public DnsBatchResult<ChangeRequest> getChangeRequest(String zoneName, String ch
203203
public DnsBatchResult<ChangeRequest> applyChangeRequest(String zoneName,
204204
ChangeRequestInfo changeRequest, Dns.ChangeRequestOption... options) {
205205
DnsBatchResult<ChangeRequest> result = new DnsBatchResult<>();
206-
RpcBatch.Callback<Change> callback = createChangeRequestCallback(zoneName, result, false);
206+
RpcBatch.Callback<Change> callback =
207+
createChangeRequestCallback(zoneName, result, false, false);
207208
Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
208209
batch.addApplyChangeRequest(zoneName, changeRequest.toPb(), callback, optionMap);
209210
return result;
210211
}
211212

212213
/**
213-
* Submits this batch for processing using a single HTTP request.
214+
* Submits this batch for processing using a single RPC request.
214215
*/
215216
public void submit() {
216217
batch.submit();
@@ -259,7 +260,7 @@ public void onFailure(GoogleJsonError googleJsonError) {
259260
* A joint callback for both "get zone" and "create zone" operations.
260261
*/
261262
private RpcBatch.Callback<ManagedZone> createZoneCallback(final DnsOptions serviceOptions,
262-
final DnsBatchResult<Zone> result, final boolean idempotent) {
263+
final DnsBatchResult<Zone> result, final boolean nullForNotFound, final boolean idempotent) {
263264
return new RpcBatch.Callback<ManagedZone>() {
264265
@Override
265266
public void onSuccess(ManagedZone response) {
@@ -268,7 +269,12 @@ public void onSuccess(ManagedZone response) {
268269

269270
@Override
270271
public void onFailure(GoogleJsonError googleJsonError) {
271-
result.error(new DnsException(googleJsonError, idempotent));
272+
DnsException serviceException = new DnsException(googleJsonError, idempotent);
273+
if (nullForNotFound && serviceException.code() == HTTP_NOT_FOUND) {
274+
result.success(null);
275+
} else {
276+
result.error(serviceException);
277+
}
272278
}
273279
};
274280
}
@@ -337,17 +343,29 @@ public void onFailure(GoogleJsonError googleJsonError) {
337343
* A joint callback for both "get change request" and "create change request" operations.
338344
*/
339345
private RpcBatch.Callback<Change> createChangeRequestCallback(final String zoneName,
340-
final DnsBatchResult<ChangeRequest> result, final boolean idempotent) {
346+
final DnsBatchResult<ChangeRequest> result, final boolean nullForNotFound,
347+
final boolean idempotent) {
341348
return new RpcBatch.Callback<Change>() {
342349
@Override
343350
public void onSuccess(Change response) {
344-
result.success(response == null ? null : ChangeRequest.fromPb(options.service(),
345-
zoneName, response));
351+
result.success(response == null ? null : ChangeRequest.fromPb(options.service(), zoneName,
352+
response));
346353
}
347354

348355
@Override
349356
public void onFailure(GoogleJsonError googleJsonError) {
350-
result.error(new DnsException(googleJsonError, idempotent));
357+
DnsException serviceException = new DnsException(googleJsonError, idempotent);
358+
if (nullForNotFound && serviceException.code() == HTTP_NOT_FOUND) {
359+
if ("entity.parameters.changeId".equals(serviceException.location())
360+
|| (serviceException.getMessage() != null
361+
&& serviceException.getMessage().contains("parameters.changeId"))) {
362+
// the change id was not found, but the zone exists
363+
result.success(null);
364+
return;
365+
}
366+
// the zone does not exist, so throw an exception
367+
}
368+
result.error(serviceException);
351369
}
352370
};
353371
}

‎gcloud-java-dns/src/main/java/com/google/cloud/dns/spi/RpcBatch.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ void addApplyChangeRequest(String zoneName, Change change, Callback<Change> call
110110
Map<DnsRpc.Option, ?> options);
111111

112112
/**
113-
* Submits a batch of requests for processing using a single HTTP request to Cloud DNS.
113+
* Submits a batch of requests for processing using a single RPC request to Cloud DNS.
114114
*/
115115
void submit();
116116
}

‎gcloud-java-dns/src/test/java/com/google/cloud/dns/DnsBatchTest.java‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.junit.Assert.assertEquals;
2020
import static org.junit.Assert.assertFalse;
2121
import static org.junit.Assert.assertNotNull;
22+
import static org.junit.Assert.assertNull;
2223
import static org.junit.Assert.assertSame;
2324
import static org.junit.Assert.assertTrue;
2425
import static org.junit.Assert.fail;
@@ -218,7 +219,9 @@ public void testCreateZone() {
218219
}
219220
// testing error here, success is tested with options
220221
RpcBatch.Callback<ManagedZone> capturedCallback = callback.getValue();
221-
capturedCallback.onFailure(GOOGLE_JSON_ERROR);
222+
GoogleJsonError error = new GoogleJsonError();
223+
error.setCode(404);
224+
capturedCallback.onFailure(error);
222225
try {
223226
batchResult.get();
224227
fail("Should throw a DnsException on error.");
@@ -279,6 +282,23 @@ public void testGetZone() {
279282
}
280283
}
281284

285+
@Test
286+
public void testGetZoneNotFound() {
287+
EasyMock.reset(batchMock);
288+
Capture<RpcBatch.Callback<ManagedZone>> callback = Capture.newInstance();
289+
Capture<Map<DnsRpc.Option, Object>> capturedOptions = Capture.newInstance();
290+
batchMock.addGetZone(EasyMock.eq(ZONE_NAME), EasyMock.capture(callback),
291+
EasyMock.capture(capturedOptions));
292+
EasyMock.replay(batchMock);
293+
DnsBatchResult<Zone> batchResult = dnsBatch.getZone(ZONE_NAME);
294+
assertEquals(0, capturedOptions.getValue().size());
295+
GoogleJsonError error = new GoogleJsonError();
296+
error.setCode(404);
297+
RpcBatch.Callback<ManagedZone> capturedCallback = callback.getValue();
298+
capturedCallback.onFailure(error);
299+
assertNull(batchResult.get());
300+
}
301+
282302
@Test
283303
public void testGetZoneWithOptions() {
284304
EasyMock.reset(dns, batchMock, optionsMock);
@@ -556,6 +576,29 @@ public void testGetChangeRequest() {
556576
}
557577
}
558578

579+
@Test
580+
public void testGetChangeRequestNotFound() {
581+
EasyMock.reset(batchMock);
582+
Capture<RpcBatch.Callback<Change>> callback = Capture.newInstance();
583+
Capture<Map<DnsRpc.Option, Object>> capturedOptions = Capture.newInstance();
584+
batchMock.addGetChangeRequest(EasyMock.eq(ZONE_NAME),
585+
EasyMock.eq(CHANGE_REQUEST_COMPLETE.generatedId()), EasyMock.capture(callback),
586+
EasyMock.capture(capturedOptions));
587+
EasyMock.replay(batchMock);
588+
DnsBatchResult<ChangeRequest> batchResult = dnsBatch.getChangeRequest(ZONE_NAME,
589+
CHANGE_REQUEST_COMPLETE.generatedId());
590+
assertEquals(0, capturedOptions.getValue().size());
591+
RpcBatch.Callback<Change> capturedCallback = callback.getValue();
592+
GoogleJsonError error = new GoogleJsonError();
593+
GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo();
594+
errorInfo.setReason("reason");
595+
errorInfo.setLocation("entity.parameters.changeId");
596+
error.setCode(404);
597+
error.setErrors(ImmutableList.of(errorInfo));
598+
capturedCallback.onFailure(error);
599+
assertNull(batchResult.get());
600+
}
601+
559602
@Test
560603
public void testGetChangeRequestWithOptions() {
561604
EasyMock.reset(dns, batchMock, optionsMock);
@@ -599,7 +642,9 @@ public void testApplyChangeRequest() {
599642
}
600643
// testing error here, success is tested with options
601644
RpcBatch.Callback<Change> capturedCallback = callback.getValue();
602-
capturedCallback.onFailure(GOOGLE_JSON_ERROR);
645+
GoogleJsonError error = new GoogleJsonError();
646+
error.setCode(404);
647+
capturedCallback.onFailure(error);
603648
try {
604649
batchResult.get();
605650
fail("Should throw a DnsException on error.");

0 commit comments

Comments
 (0)