Make DNS batch get zone/changerequest return null for NOT_FOUND

Author: mziccard

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

@@ -135,8 +135,21 @@ public BaseServiceException(IOException exception, boolean idempotent) {
     this.debugInfo = debugInfo;
   }
 
-  public BaseServiceException(GoogleJsonError error, boolean idempotent) {
-    this(error.getCode(), error.getMessage(), reason(error), idempotent);
+  public BaseServiceException(GoogleJsonError googleJsonError, boolean idempotent) {
+    super(googleJsonError.getMessage());
+    Error error = new Error(googleJsonError.getCode(), reason(googleJsonError));
+    this.code = error.code;
+    this.reason = error.reason;
+    this.retryable = isRetryable(idempotent, error);
+    if (this.reason != null) {
+      GoogleJsonError.ErrorInfo errorInfo = googleJsonError.getErrors().get(0);
+      this.location = errorInfo.getLocation();
+      this.debugInfo = (String) errorInfo.get("debugInfo");
+    } else {
+      this.location = null;
+      this.debugInfo = null;
+    }
+    this.idempotent = idempotent;
   }
 
   public BaseServiceException(int code, String message, String reason, boolean idempotent) {

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

@@ -91,7 +91,7 @@ public DnsBatchResult<Page<Zone>> listZones(Dns.ZoneListOption... options) {
   public DnsBatchResult<Zone> createZone(ZoneInfo zone, Dns.ZoneOption... options) {
     DnsBatchResult<Zone> result = new DnsBatchResult<>();
     // todo this can cause misleading report of a failure, intended to be fixed within #924
-    RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, true);
+    RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, false, true);
     Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
     batch.addCreateZone(zone.toPb(), callback, optionMap);
     return result;
@@ -118,7 +118,7 @@ public DnsBatchResult<Boolean> deleteZone(String zoneName) {
    */
   public DnsBatchResult<Zone> getZone(String zoneName, Dns.ZoneOption... options) {
     DnsBatchResult<Zone> result = new DnsBatchResult<>();
-    RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, true);
+    RpcBatch.Callback<ManagedZone> callback = createZoneCallback(this.options, result, true, true);
     Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
     batch.addGetZone(zoneName, callback, optionMap);
     return result;
@@ -186,7 +186,7 @@ public DnsBatchResult<Page<ChangeRequest>> listChangeRequests(String zoneName,
   public DnsBatchResult<ChangeRequest> getChangeRequest(String zoneName, String changeRequestId,
       Dns.ChangeRequestOption... options) {
     DnsBatchResult<ChangeRequest> result = new DnsBatchResult<>();
-    RpcBatch.Callback<Change> callback = createChangeRequestCallback(zoneName, result, true);
+    RpcBatch.Callback<Change> callback = createChangeRequestCallback(zoneName, result, true, true);
     Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
     batch.addGetChangeRequest(zoneName, changeRequestId, callback, optionMap);
     return result;
@@ -203,14 +203,15 @@ public DnsBatchResult<ChangeRequest> getChangeRequest(String zoneName, String ch
   public DnsBatchResult<ChangeRequest> applyChangeRequest(String zoneName,
       ChangeRequestInfo changeRequest, Dns.ChangeRequestOption... options) {
     DnsBatchResult<ChangeRequest> result = new DnsBatchResult<>();
-    RpcBatch.Callback<Change> callback = createChangeRequestCallback(zoneName, result, false);
+    RpcBatch.Callback<Change> callback =
+        createChangeRequestCallback(zoneName, result, false, false);
     Map<DnsRpc.Option, ?> optionMap = DnsImpl.optionMap(options);
     batch.addApplyChangeRequest(zoneName, changeRequest.toPb(), callback, optionMap);
     return result;
   }
 
   /**
-   * Submits this batch for processing using a single HTTP request.
+   * Submits this batch for processing using a single RPC request.
    */
   public void submit() {
     batch.submit();
@@ -259,7 +260,7 @@ public void onFailure(GoogleJsonError googleJsonError) {
    * A joint callback for both "get zone" and "create zone" operations.
    */
   private RpcBatch.Callback<ManagedZone> createZoneCallback(final DnsOptions serviceOptions,
-      final DnsBatchResult<Zone> result, final boolean idempotent) {
+      final DnsBatchResult<Zone> result, final boolean nullForNotFound, final boolean idempotent) {
     return new RpcBatch.Callback<ManagedZone>() {
       @Override
       public void onSuccess(ManagedZone response) {
@@ -268,7 +269,12 @@ public void onSuccess(ManagedZone response) {
 
       @Override
       public void onFailure(GoogleJsonError googleJsonError) {
-        result.error(new DnsException(googleJsonError, idempotent));
+        DnsException serviceException = new DnsException(googleJsonError, idempotent);
+        if (nullForNotFound && serviceException.code() == HTTP_NOT_FOUND) {
+          result.success(null);
+        } else {
+          result.error(serviceException);
+        }
       }
     };
   }
@@ -337,17 +343,28 @@ public void onFailure(GoogleJsonError googleJsonError) {
    * A joint callback for both "get change request" and "create change request" operations.
    */
   private RpcBatch.Callback<Change> createChangeRequestCallback(final String zoneName,
-      final DnsBatchResult<ChangeRequest> result, final boolean idempotent) {
+      final DnsBatchResult<ChangeRequest> result, final boolean nullForNotFound,
+      final boolean idempotent) {
     return new RpcBatch.Callback<Change>() {
       @Override
       public void onSuccess(Change response) {
-        result.success(response == null ? null : ChangeRequest.fromPb(options.service(),
-            zoneName, response));
+        result.success(response == null ? null : ChangeRequest.fromPb(options.service(), zoneName,
+            response));
       }
 
       @Override
       public void onFailure(GoogleJsonError googleJsonError) {
-        result.error(new DnsException(googleJsonError, idempotent));
+        DnsException serviceException = new DnsException(googleJsonError, idempotent);
+        if (nullForNotFound && serviceException.code() == HTTP_NOT_FOUND
+            && ("entity.parameters.changeId".equals(serviceException.location())
+            || serviceException.getMessage() != null
+            && serviceException.getMessage().contains("parameters.changeId"))) {
+          // the change id was not found, but the zone exists
+          result.success(null);
+        } else {
+          // the zone does not exist, so throw an exception
+          result.error(serviceException);
+        }
       }
     };
   }

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

@@ -331,15 +331,14 @@ public Change getChangeRequest(String zoneName, String changeRequestId, Map<Opti
       return getChangeRequestCall(zoneName, changeRequestId, options).execute();
     } catch (IOException ex) {
       DnsException serviceException = translate(ex, true);
-      if (serviceException.code() == HTTP_NOT_FOUND) {
-        if ("entity.parameters.changeId".equals(serviceException.location())
-            || (serviceException.getMessage() != null
-            && serviceException.getMessage().contains("parameters.changeId"))) {
-          // the change id was not found, but the zone exists
-          return null;
-        }
-        // the zone does not exist, so throw an exception
+      if (serviceException.code() == HTTP_NOT_FOUND
+          && ("entity.parameters.changeId".equals(serviceException.location())
+          || serviceException.getMessage() != null
+          && serviceException.getMessage().contains("parameters.changeId"))) {
+        // the change id was not found, but the zone exists
+        return null;
       }
+      // the zone does not exist, so throw an exception
       throw serviceException;
     }
   }

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

@@ -110,7 +110,7 @@ void addApplyChangeRequest(String zoneName, Change change, Callback<Change> call
       Map<DnsRpc.Option, ?> options);
 
   /**
-   * Submits a batch of requests for processing using a single HTTP request to Cloud DNS.
+   * Submits a batch of requests for processing using a single RPC request to Cloud DNS.
    */
   void submit();
 }

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

@@ -19,6 +19,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -218,7 +219,9 @@ public void testCreateZone() {
     }
     // testing error here, success is tested with options
     RpcBatch.Callback<ManagedZone> capturedCallback = callback.getValue();
-    capturedCallback.onFailure(GOOGLE_JSON_ERROR);
+    GoogleJsonError error = new GoogleJsonError();
+    error.setCode(404);
+    capturedCallback.onFailure(error);
     try {
       batchResult.get();
       fail("Should throw a DnsException on error.");
@@ -279,6 +282,23 @@ public void testGetZone() {
     }
   }
 
+  @Test
+  public void testGetZoneNotFound() {
+    EasyMock.reset(batchMock);
+    Capture<RpcBatch.Callback<ManagedZone>> callback = Capture.newInstance();
+    Capture<Map<DnsRpc.Option, Object>> capturedOptions = Capture.newInstance();
+    batchMock.addGetZone(EasyMock.eq(ZONE_NAME), EasyMock.capture(callback),
+        EasyMock.capture(capturedOptions));
+    EasyMock.replay(batchMock);
+    DnsBatchResult<Zone> batchResult = dnsBatch.getZone(ZONE_NAME);
+    assertEquals(0, capturedOptions.getValue().size());
+    GoogleJsonError error = new GoogleJsonError();
+    error.setCode(404);
+    RpcBatch.Callback<ManagedZone> capturedCallback = callback.getValue();
+    capturedCallback.onFailure(error);
+    assertNull(batchResult.get());
+  }
+
   @Test
   public void testGetZoneWithOptions() {
     EasyMock.reset(dns, batchMock, optionsMock);
@@ -556,6 +576,29 @@ public void testGetChangeRequest() {
     }
   }
 
+  @Test
+  public void testGetChangeRequestNotFound() {
+    EasyMock.reset(batchMock);
+    Capture<RpcBatch.Callback<Change>> callback = Capture.newInstance();
+    Capture<Map<DnsRpc.Option, Object>> capturedOptions = Capture.newInstance();
+    batchMock.addGetChangeRequest(EasyMock.eq(ZONE_NAME),
+        EasyMock.eq(CHANGE_REQUEST_COMPLETE.generatedId()), EasyMock.capture(callback),
+        EasyMock.capture(capturedOptions));
+    EasyMock.replay(batchMock);
+    DnsBatchResult<ChangeRequest> batchResult = dnsBatch.getChangeRequest(ZONE_NAME,
+        CHANGE_REQUEST_COMPLETE.generatedId());
+    assertEquals(0, capturedOptions.getValue().size());
+    RpcBatch.Callback<Change> capturedCallback = callback.getValue();
+    GoogleJsonError error = new GoogleJsonError();
+    GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo();
+    errorInfo.setReason("reason");
+    errorInfo.setLocation("entity.parameters.changeId");
+    error.setCode(404);
+    error.setErrors(ImmutableList.of(errorInfo));
+    capturedCallback.onFailure(error);
+    assertNull(batchResult.get());
+  }
+
   @Test
   public void testGetChangeRequestWithOptions() {
     EasyMock.reset(dns, batchMock, optionsMock);
@@ -599,7 +642,9 @@ public void testApplyChangeRequest() {
     }
     // testing error here, success is tested with options
     RpcBatch.Callback<Change> capturedCallback = callback.getValue();
-    capturedCallback.onFailure(GOOGLE_JSON_ERROR);
+    GoogleJsonError error = new GoogleJsonError();
+    error.setCode(404);
+    capturedCallback.onFailure(error);
     try {
       batchResult.get();
       fail("Should throw a DnsException on error.");