Merge branch 'stable-3.2'

* stable-3.2:
  Fix JSON audit user reporting and testing
  Fix JSON rendering of HTTP response audit

Change-Id: I2b49b653a8234f1e8e46d96f21a3f3bcd6f5466d
diff --git a/src/main/java/com/googlesource/gerrit/plugins/auditsl4j/AuditRendererToJson.java b/src/main/java/com/googlesource/gerrit/plugins/auditsl4j/AuditRendererToJson.java
index 2ea29f3..2c4133b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/auditsl4j/AuditRendererToJson.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/auditsl4j/AuditRendererToJson.java
@@ -16,6 +16,7 @@
 
 import com.google.common.collect.ListMultimap;
 import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.json.OutputFormat;
 import com.google.gerrit.server.AccessPath;
 import com.google.gerrit.server.AuditEvent;
@@ -23,6 +24,11 @@
 import com.google.gson.ExclusionStrategy;
 import com.google.gson.FieldAttributes;
 import com.google.gson.Gson;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonSerializationContext;
+import com.google.gson.JsonSerializer;
+import java.lang.reflect.Type;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Optional;
@@ -58,14 +64,38 @@
               && !CurrentUser.class.isAssignableFrom(clazz)
               && !ListMultimap.class.isAssignableFrom(clazz)
               && !AuditEvent.UUID.class.isAssignableFrom(clazz)
+              && !Response.class.isAssignableFrom(clazz)
               && !WHITELIST_CLASSES.contains(clazz);
         }
       };
 
+  private static final JsonSerializer<CurrentUser> CURRENT_USER_SERIALIZER =
+      new JsonSerializer<CurrentUser>() {
+
+        @Override
+        public JsonElement serialize(
+            CurrentUser user, Type typeOfSrc, JsonSerializationContext context) {
+          JsonObject jsonUser = new JsonObject();
+
+          jsonUser.addProperty("access_path", user.getAccessPath().name());
+          jsonUser.addProperty("name", user.getLoggableName());
+          jsonUser.addProperty("internal_user", user.isInternalUser());
+          jsonUser.addProperty("identified_user", user.isIdentifiedUser());
+          jsonUser.addProperty("impersonating", user.isImpersonating());
+
+          if (user.isIdentifiedUser()) {
+            jsonUser.addProperty("account_id", user.asIdentifiedUser().getAccountId().get());
+          }
+
+          return jsonUser;
+        }
+      };
+
   private final Gson gson =
       OutputFormat.JSON_COMPACT
           .newGsonBuilder()
           .setExclusionStrategies(INCLUDE_ONLY_WHITELISTED)
+          .registerTypeAdapter(CurrentUser.class, CURRENT_USER_SERIALIZER)
           .create();
 
   @Override
diff --git a/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToCsvTest.java b/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToCsvTest.java
index de89e0d..bd68513 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToCsvTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToCsvTest.java
@@ -22,10 +22,7 @@
 import com.google.gerrit.common.Version;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.server.audit.AuditListener;
-import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.inject.AbstractModule;
-import com.google.inject.Inject;
-import org.apache.http.client.fluent.Request;
 import org.junit.Test;
 
 @Sandboxed
@@ -34,8 +31,6 @@
     sysModule = "com.googlesource.gerrit.plugins.auditsl4j.LoggerAuditToCsvTest$TestModule")
 public class LoggerAuditToCsvTest extends LightweightPluginDaemonTest implements WaitForCondition {
 
-  @Inject @CanonicalWebUrl private String webUrl;
-
   public static class TestModule extends AbstractModule {
 
     @Override
@@ -50,12 +45,37 @@
   public void testHttpCsvAudit() throws Exception {
     AuditWriterToStringList auditStrings = getPluginInstance(AuditWriterToStringList.class);
 
-    Request.Get(webUrl + "config/server/version").execute().returnResponse();
+    anonymousRestSession.get("/config/server/version").assertOK();
 
-    assertThat(waitFor(() -> auditStrings.strings.size() == 2)).isTrue();
+    waitForFirstAuditRecord(auditStrings);
     assertThat(auditStrings.strings.get(1)).contains(Version.getVersion());
   }
 
+  @Test
+  public void testHttpCsvAuditShouldContainCurrentUser() throws Exception {
+    AuditWriterToStringList auditStrings = getPluginInstance(AuditWriterToStringList.class);
+
+    anonymousRestSession.get("/config/server/version").assertOK();
+
+    waitForFirstAuditRecord(auditStrings);
+    assertThat(auditStrings.strings.get(1)).contains("ANONYMOUS");
+  }
+
+  @Test
+  public void testHttpCsvAuditShouldContainIdentifiedUser() throws Exception {
+    AuditWriterToStringList auditStrings = getPluginInstance(AuditWriterToStringList.class);
+
+    userRestSession.get("/config/server/version").assertOK();
+
+    waitForFirstAuditRecord(auditStrings);
+
+    assertThat(auditStrings.strings.get(1).toLowerCase()).contains(user.id().toString());
+  }
+
+  private void waitForFirstAuditRecord(AuditWriterToStringList auditStrings) {
+    assertThat(waitFor(() -> auditStrings.strings.size() >= 2)).isTrue();
+  }
+
   private <T> T getPluginInstance(Class<T> clazz) {
     return plugin.getSysInjector().getInstance(clazz);
   }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToJsonTest.java b/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToJsonTest.java
index 9d0e48c..41dd9b3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToJsonTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/auditsl4j/LoggerAuditToJsonTest.java
@@ -27,7 +27,6 @@
 import com.google.gson.JsonObject;
 import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
-import org.apache.http.client.fluent.Request;
 import org.junit.Test;
 
 @Sandboxed
@@ -52,17 +51,46 @@
   public void testHttpJsonAudit() throws Exception {
     AuditWriterToStringList auditStrings = getPluginInstance(AuditWriterToStringList.class);
 
-    Request.Get(webUrl + "config/server/version").execute().returnResponse();
-
-    assertThat(waitFor(() -> auditStrings.strings.size() == 1)).isTrue();
+    anonymousRestSession.get("/config/server/version").assertOK();
+    waitForFirstAuditRecord(auditStrings);
 
     String auditJsonString = auditStrings.strings.get(0);
+    assertValidJson(auditJsonString);
     assertThat(auditJsonString).contains(Version.getVersion());
-    JsonObject auditJson = new Gson().fromJson(auditJsonString, JsonObject.class);
-    assertThat(auditJson).isNotNull();
+  }
+
+  @Test
+  public void testHttpCsvAuditShouldContainAnonymousUser() throws Exception {
+    AuditWriterToStringList auditStrings = getPluginInstance(AuditWriterToStringList.class);
+
+    anonymousRestSession.get("/config/server/version").assertOK();
+    waitForFirstAuditRecord(auditStrings);
+
+    assertThat(auditStrings.strings.get(0).toLowerCase()).contains("anonymous");
+  }
+
+  @Test
+  public void testHttpCsvAuditShouldContainIdentifiedUser() throws Exception {
+    AuditWriterToStringList auditStrings = getPluginInstance(AuditWriterToStringList.class);
+
+    userRestSession.get("/config/server/version").assertOK();
+
+    waitForFirstAuditRecord(auditStrings);
+
+    assertThat(auditStrings.strings.get(0).toLowerCase()).contains(user.id().toString());
   }
 
   private <T> T getPluginInstance(Class<T> clazz) {
     return plugin.getSysInjector().getInstance(clazz);
   }
+
+  private void waitForFirstAuditRecord(AuditWriterToStringList auditStrings) {
+    assertThat(waitFor(() -> auditStrings.strings.size() >= 1)).isTrue();
+  }
+
+  private JsonObject assertValidJson(String auditJsonString) {
+    JsonObject auditJson = new Gson().fromJson(auditJsonString, JsonObject.class);
+    assertThat(auditJson).isNotNull();
+    return auditJson;
+  }
 }