From f01ded3fa79bcff6160fb0fe96e37575f3825120 Mon Sep 17 00:00:00 2001
From: mcilwain <mcilwain@google.com>
Date: Fri, 1 Feb 2019 10:46:28 -0800
Subject: [PATCH] Make logged response in NordnUploadAction intelligible

Right now it's logging the raw bytes, which look like:

response data: [65, 117, 116, 104, 111, 114, 105, 122, 97, 116, 105, 111, 110, 32, 114, 101, 113, 117, 105, 114, 101, 100]

We'd rather convert it to ASCII characters (what the NORDN service uses) before
logging it, so that it'd instead look like:

response data: Authorization required

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=231998658
---
 java/google/registry/tmch/NordnUploadAction.java      | 11 ++++++++---
 .../google/registry/tmch/NordnUploadActionTest.java   | 11 +++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/java/google/registry/tmch/NordnUploadAction.java b/java/google/registry/tmch/NordnUploadAction.java
index 9033ccf19..2af5261c6 100644
--- a/java/google/registry/tmch/NordnUploadAction.java
+++ b/java/google/registry/tmch/NordnUploadAction.java
@@ -26,6 +26,7 @@ import static google.registry.tmch.LordnTaskUtils.COLUMNS_CLAIMS;
 import static google.registry.tmch.LordnTaskUtils.COLUMNS_SUNRISE;
 import static google.registry.util.UrlFetchUtils.getHeaderFirst;
 import static google.registry.util.UrlFetchUtils.setPayloadMultipart;
+import static java.nio.charset.StandardCharsets.US_ASCII;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static javax.servlet.http.HttpServletResponse.SC_ACCEPTED;
 
@@ -191,9 +192,13 @@ public final class NordnUploadAction implements Runnable {
     lordnRequestInitializer.initialize(req, tld);
     setPayloadMultipart(req, "file", "claims.csv", CSV_UTF_8, csvData, random);
     HTTPResponse rsp = fetchService.fetch(req);
-    logger.atInfo().log(
-        "LORDN upload task %s response: HTTP response code %d, response data: %s",
-        actionLogId, rsp.getResponseCode(), rsp.getContent());
+    if (logger.atInfo().isEnabled()) {
+      String response =
+          (rsp.getContent() == null) ? "(null)" : new String(rsp.getContent(), US_ASCII);
+      logger.atInfo().log(
+          "LORDN upload task %s response: HTTP response code %d, response data: %s",
+          actionLogId, rsp.getResponseCode(), response);
+    }
     if (rsp.getResponseCode() != SC_ACCEPTED) {
       throw new UrlFetchException(
           String.format(
diff --git a/javatests/google/registry/tmch/NordnUploadActionTest.java b/javatests/google/registry/tmch/NordnUploadActionTest.java
index 86534e690..feafadba8 100644
--- a/javatests/google/registry/tmch/NordnUploadActionTest.java
+++ b/javatests/google/registry/tmch/NordnUploadActionTest.java
@@ -28,6 +28,7 @@ import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.JUnitBackports.assertThrows;
 import static google.registry.testing.TaskQueueHelper.assertTasksEnqueued;
 import static google.registry.util.UrlFetchUtils.getHeaderFirst;
+import static java.nio.charset.StandardCharsets.US_ASCII;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static javax.servlet.http.HttpServletResponse.SC_ACCEPTED;
 import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
@@ -112,6 +113,7 @@ public class NordnUploadActionTest {
   public void before() throws Exception {
     inject.setStaticField(Ofy.class, "clock", clock);
     when(fetchService.fetch(any(HTTPRequest.class))).thenReturn(httpResponse);
+    when(httpResponse.getContent()).thenReturn("Success".getBytes(US_ASCII));
     when(httpResponse.getResponseCode()).thenReturn(SC_ACCEPTED);
     when(httpResponse.getHeadersUncombined())
         .thenReturn(ImmutableList.of(new HTTPHeader(LOCATION, "http://trololol")));
@@ -240,6 +242,15 @@ public class NordnUploadActionTest {
     assertThat(new String(getCapturedHttpRequest().getPayload(), UTF_8)).contains(SUNRISE_CSV);
   }
 
+  @Test
+  public void test_noResponseContent_stillWorksNormally() throws Exception {
+    // Returning null only affects logging.
+    when(httpResponse.getContent()).thenReturn(null);
+    persistSunriseModeDomain();
+    action.run();
+    assertThat(new String(getCapturedHttpRequest().getPayload(), UTF_8)).contains(SUNRISE_CSV);
+  }
+
   @Test
   public void testRun_sunriseMode_verifyTaskGetsEnqueuedWithSunriseCsv() {
     persistSunriseModeDomain();