Fix legacy logging bug from FlowRunner

We've moved completely to the JSON based reporting framework. The legacy logging statement is only for human consumption, therefore removing the comments. Also fixes a bug where the last argument is not used due to the formatter only expecting 7 arguments.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=198558998

java/google/registry/flows/FlowRunner.java

@@ -35,10 +35,7 @@ import javax.inject.Provider;
 /** Run a flow, either transactionally or not, with logging and retrying as needed. */
 public class FlowRunner {
 
-  /** Log format used by legacy ICANN reporting parsing - DO NOT CHANGE. */
+  private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 8);
-  // TODO(b/20725722): remove this log format entirely once we've transitioned to using the
-  //   JSON log line below instead, or change this one to be for human consumption only.
-  private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 7);
 
   private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
 
@@ -59,10 +56,7 @@ public class FlowRunner {
   /** Runs the EPP flow, and records metrics on the given builder. */
   public EppOutput run(final EppMetric.Builder eppMetricBuilder) throws EppException {
     String prettyXml = prettyPrint(inputXmlBytes);
-    // This log line is very fragile since it's used for ICANN reporting - DO NOT CHANGE.
+
-    // New data to be logged should be added only to the JSON log statement below.
-    // TODO(b/20725722): remove this log statement entirely once we've transitioned to using the
-    //   log line below instead, or change this one to be for human consumption only.
     if (logger.isLoggable(Level.INFO)) {
       logger.infofmt(
           COMMAND_LOG_FORMAT,

javatests/google/registry/flows/FlowRunnerTest.java

@@ -124,7 +124,7 @@ public class FlowRunnerTest extends ShardableTestCase {
   }
 
   @Test
-  public void testRun_legacyLoggingStatement_basic() throws Exception {
+  public void testRun_loggingStatement_basic() throws Exception {
     flowRunner.run(eppMetricBuilder);
     assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t")))
         .containsExactly(
@@ -136,12 +136,13 @@ public class FlowRunnerTest extends ShardableTestCase {
             "",  // Extra newline at the end of the XML.
             "PasswordOnlyTransportCredentials{}",
             "UNIT_TEST",
-            "LIVE")
+            "LIVE",
+            "NORMAL")
         .inOrder();
   }
 
   @Test
-  public void testRun_legacyLoggingStatement_httpSessionMetadata() throws Exception {
+  public void testRun_loggingStatement_httpSessionMetadata() throws Exception {
     flowRunner.sessionMetadata = new HttpSessionMetadata(new FakeHttpSession());
     flowRunner.sessionMetadata.setClientId("TheRegistrar");
     flowRunner.run(eppMetricBuilder);
@@ -152,7 +153,7 @@ public class FlowRunnerTest extends ShardableTestCase {
   }
 
   @Test
-  public void testRun_legacyLoggingStatement_gaeUserCredentials() throws Exception {
+  public void testRun_loggingStatement_gaeUserCredentials() throws Exception {
     flowRunner.credentials =
         GaeUserCredentials.forTestingUser(new User("user@example.com", "authDomain"), false);
     flowRunner.run(eppMetricBuilder);
@@ -161,7 +162,7 @@ public class FlowRunnerTest extends ShardableTestCase {
   }
 
   @Test
-  public void testRun_legacyLoggingStatement_tlsCredentials() throws Exception {
+  public void testRun_loggingStatement_tlsCredentials() throws Exception {
     flowRunner.credentials = new TlsCredentials("abc123def", Optional.of("127.0.0.1"), "sni");
     flowRunner.run(eppMetricBuilder);
     assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t")))
@@ -170,7 +171,7 @@ public class FlowRunnerTest extends ShardableTestCase {
   }
 
   @Test
-  public void testRun_legacyLoggingStatement_dryRun() throws Exception {
+  public void testRun_loggingStatement_dryRun() throws Exception {
     flowRunner.isDryRun = true;
     flowRunner.run(eppMetricBuilder);
     assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t")))
@@ -178,14 +179,22 @@ public class FlowRunnerTest extends ShardableTestCase {
   }
 
   @Test
-  public void testRun_legacyLoggingStatement_complexEppInput() throws Exception {
+  public void testRun_loggingStatement_superuser() throws Exception {
+    flowRunner.isSuperuser = true;
+    flowRunner.run(eppMetricBuilder);
+    assertThat(Splitter.on("\n\t").split(findFirstLogMessageByPrefix(handler, "EPP Command\n\t")))
+        .contains("SUPERUSER");
+  }
+
+  @Test
+  public void testRun_loggingStatement_complexEppInput() throws Exception {
     String domainCreateXml = loadFile(getClass(), "domain_create_prettyprinted.xml");
     flowRunner.inputXmlBytes = domainCreateXml.getBytes(UTF_8);
     flowRunner.run(eppMetricBuilder);
     String logMessage = findFirstLogMessageByPrefix(handler, "EPP Command\n\t");
     List<String> lines = Splitter.on("\n\t").splitToList(logMessage);
     assertThat(lines.size()).named("number of lines in log message").isAtLeast(9);
-    String xml = Joiner.on('\n').join(lines.subList(3, lines.size() - 3));
+    String xml = Joiner.on('\n').join(lines.subList(3, lines.size() - 4));
     assertThat(xml).isEqualTo(domainCreateXml);
   }
 }