allow null error fields

Currently a NullPointerException is thrown due to disallowed null
values in ImmutableMap. This change changes error construction to use
Jackson data to allow for null values.

Author: tangiel

endpoints-framework/src/main/java/com/google/api/server/spi/response/RestResponseResultWriter.java

@@ -15,6 +15,10 @@
  */
 package com.google.api.server.spi.response;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.api.server.spi.ObjectMapperUtil;
 import com.google.api.server.spi.ServiceException;
 import com.google.api.server.spi.config.model.ApiSerializationConfig;
 import com.google.common.base.Strings;
@@ -31,12 +35,14 @@
  */
 public class RestResponseResultWriter extends ServletResponseResultWriter {
   private final boolean enableExceptionCompatibility;
+  private final ObjectMapper objectMapper;
 
   public RestResponseResultWriter(
       HttpServletResponse servletResponse, ApiSerializationConfig serializationConfig,
       boolean prettyPrint, boolean enableExceptionCompatibility) {
     super(servletResponse, serializationConfig, prettyPrint);
     this.enableExceptionCompatibility = enableExceptionCompatibility;
+    this.objectMapper = ObjectMapperUtil.createStandardObjectMapper(serializationConfig);
   }
 
   /**
@@ -69,15 +75,16 @@ public void writeError(ServiceException e) throws IOException {
   }
 
   private Object createError(int code, String reason, String domain, String message) {
-    return ImmutableMap.of(
-        "error", ImmutableMap.of(
-            "errors", ImmutableList.of(ImmutableMap.of(
-                "domain", domain,
-                "reason", reason,
-                "message", message
-            )),
-            "code", code,
-            "message", message
-        ));
+    ObjectNode topLevel = objectMapper.createObjectNode();
+    ObjectNode topError = objectMapper.createObjectNode();
+    ObjectNode error = objectMapper.createObjectNode();
+    error.put("domain", domain);
+    error.put("reason", reason);
+    error.put("message", message);
+    topError.set("errors", objectMapper.createArrayNode().add(error));
+    topError.put("code", code);
+    topError.put("message", message);
+    topLevel.set("error", topError);
+    return topLevel;
   }
 }

endpoints-framework/src/test/java/com/google/api/server/spi/response/RestResponseResultWriterTest.java

@@ -47,7 +47,8 @@ public void writeError_401() throws Exception {
   @Test
   public void writeError_402() throws Exception {
     doWriteErrorTest(402 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "paymentRequired" /* reason */);
+        "unsupportedProtocol" /* compatReason */, "paymentRequired" /* reason */,
+        "error" /* message */);
   }
 
   @Test
@@ -68,19 +69,21 @@ public void writeError_405() throws Exception {
   @Test
   public void writeError_406() throws Exception {
     doWriteErrorTest(406 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "notAcceptable" /* reason */);
+        "unsupportedProtocol" /* compatReason */, "notAcceptable" /* reason */,
+        "error" /* message */);
   }
 
   @Test
   public void writeError_407() throws Exception {
     doWriteErrorTest(407 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "proxyAuthentication" /* reason */);
+        "unsupportedProtocol" /* compatReason */, "proxyAuthentication" /* reason */,
+        "error" /* message */);
   }
 
   @Test
   public void writeError_408() throws Exception {
     doWriteErrorTest(408 /* exceptionCode */, 503 /* expectedCompatCode */,
-        "backendError" /* compatReason */, "requestTimeout" /* reason */);
+        "backendError" /* compatReason */, "requestTimeout" /* reason */, "error" /* message */);
   }
 
   @Test
@@ -96,7 +99,8 @@ public void writeError_410() throws Exception {
   @Test
   public void writeError_411() throws Exception {
     doWriteErrorTest(411 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "lengthRequired" /* reason */);
+        "unsupportedProtocol" /* compatReason */, "lengthRequired" /* reason */,
+        "error" /* message */);
   }
 
   @Test
@@ -112,25 +116,25 @@ public void writeError_413() throws Exception {
   @Test
   public void writeError_414() throws Exception {
     doWriteErrorTest(414 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "uriTooLong" /* reason */);
+        "unsupportedProtocol" /* compatReason */, "uriTooLong" /* reason */, "error" /* message */);
   }
 
   @Test
   public void writeError_415() throws Exception {
     doWriteErrorTest(415 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "unsupportedMediaType");
+        "unsupportedProtocol" /* compatReason */, "unsupportedMediaType", "error" /* message */);
   }
 
   @Test
   public void writeError_416() throws Exception {
     doWriteErrorTest(416 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "rangeNotSatisfiable");
+        "unsupportedProtocol" /* compatReason */, "rangeNotSatisfiable", "error" /* message */);
   }
 
   @Test
   public void writeError_417() throws Exception {
     doWriteErrorTest(417 /* exceptionCode */, 404 /* expectedCompatCode */,
-        "unsupportedProtocol" /* compatReason */, "expectationFailed");
+        "unsupportedProtocol" /* compatReason */, "expectationFailed", "error" /* message */);
   }
 
   @Test
@@ -141,40 +145,54 @@ public void writeError_500s() throws Exception {
     }
   }
 
+  @Test
+  public void writeError_nullMessage() throws Exception {
+    doWriteErrorTest(500 /* exceptionCode */, 503 /* expectedCompatCode */,
+        "backendError" /* compatReason */, "backendError" /* reason */, null);
+  }
+
   /**
    * Tests that an error is translated according to Lily if specified, and the code is left alone
    * if compatibility mode is off. Both cases test for the correct error structure in the response.
    */
   private void doWriteErrorTest(int exceptionCode, int expectedCompatCode, String reason)
       throws Exception {
-    doWriteErrorTest(exceptionCode, expectedCompatCode, reason, reason);
+    doWriteErrorTest(exceptionCode, expectedCompatCode, reason, reason, "error");
   }
 
   /**
    * Tests that an error is translated according to Lily if specified, and the code is left alone
    * if compatibility mode is off. Both cases test for the correct error structure in the response.
    */
   private void doWriteErrorTest(int exceptionCode, int expectedCompatCode, String compatReason,
-      String reason) throws Exception {
-    writeError(exceptionCode, expectedCompatCode, compatReason, true);
-    writeError(exceptionCode, exceptionCode, reason, false);
+      String reason, String message) throws Exception {
+    writeError(exceptionCode, expectedCompatCode, compatReason, message, true);
+    writeError(exceptionCode, exceptionCode, reason, message, false);
   }
 
-  private void writeError(int exceptionCode, int expectedCode, String reason,
+  private void writeError(int exceptionCode, int expectedCode, String reason, String message,
       boolean enableExceptionCompatibility) throws Exception {
     MockHttpServletResponse response = new MockHttpServletResponse();
     RestResponseResultWriter writer = new RestResponseResultWriter(
         response, null, true /* prettyPrint */, enableExceptionCompatibility);
-    writer.writeError(new ServiceException(exceptionCode, "error"));
+    writer.writeError(new ServiceException(exceptionCode, message));
     ObjectMapper mapper = ObjectMapperUtil.createStandardObjectMapper();
     ObjectNode content = mapper.readValue(response.getContentAsString(), ObjectNode.class);
     JsonNode outerError = content.path("error");
     assertThat(outerError.path("code").asInt()).isEqualTo(expectedCode);
-    assertThat(outerError.path("message").asText()).isEqualTo("error");
+    if (message == null) {
+      assertThat(outerError.path("message").isNull()).isTrue();
+    } else {
+      assertThat(outerError.path("message").asText()).isEqualTo(message);
+    }
     JsonNode innerError = outerError.path("errors").path(0);
     assertThat(innerError.path("domain").asText()).isEqualTo("global");
     assertThat(innerError.path("reason").asText()).isEqualTo(reason);
-    assertThat(innerError.path("message").asText()).isEqualTo("error");
+    if (message == null) {
+      assertThat(innerError.path("message").isNull()).isTrue();
+    } else {
+      assertThat(innerError.path("message").asText()).isEqualTo(message);
+    }
   }
 
   @Test