Expand tests for plugin fields

Instead of a single plugin that does everything, use multiple separate,
simple plugins. Add separate tests for extension, REST, and SSH APIs.
This requires us to move tests to a separate class, so we can enable
HTTP and SSH. That's probably a good thing anyway, since the length has
expanded considerably.

Change-Id: Ifc883dfcc1e8e4c7bcc4e93f4274088456a508f9
diff --git a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index ba11db9..607d970 100644
--- a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -75,6 +75,8 @@
     JSON
   }
 
+  public static final Gson GSON = new Gson();
+
   private final GitRepositoryManager repoManager;
   private final ChangeQueryBuilder queryBuilder;
   private final ChangeQueryProcessor queryProcessor;
@@ -355,7 +357,7 @@
         break;
 
       case JSON:
-        out.print(new Gson().toJson(data));
+        out.print(GSON.toJson(data));
         out.print('\n');
         break;
     }
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index d7083bc..9cc277f 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -52,7 +52,6 @@
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -74,10 +73,8 @@
 import com.google.gerrit.common.data.LabelFunction;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.extensions.annotations.Exports;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerResult;
-import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
 import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
 import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -117,7 +114,6 @@
 import com.google.gerrit.extensions.common.LabelInfo;
 import com.google.gerrit.extensions.common.MergeInput;
 import com.google.gerrit.extensions.common.MergePatchSetInput;
-import com.google.gerrit.extensions.common.PluginDefinedInfo;
 import com.google.gerrit.extensions.common.PureRevertInfo;
 import com.google.gerrit.extensions.common.RevisionInfo;
 import com.google.gerrit.extensions.common.TrackingIdInfo;
@@ -150,7 +146,6 @@
 import com.google.gerrit.server.index.change.IndexedChangeQuery;
 import com.google.gerrit.server.project.testing.Util;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor.ChangeAttributeFactory;
 import com.google.gerrit.server.restapi.change.PostReview;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.BatchUpdateOp;
@@ -158,15 +153,12 @@
 import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.FakeEmailSender.Message;
 import com.google.gerrit.testing.TestTimeUtil;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -2499,67 +2491,6 @@
     assertThat(query("owner:self project:{" + project.get() + "}")).isEmpty();
   }
 
-  static class CustomAttributeModule extends AbstractModule {
-    static class MyInfo extends PluginDefinedInfo {
-      public String theAttribute;
-    }
-
-    @Override
-    public void configure() {
-      bind(ChangeAttributeFactory.class)
-          .annotatedWith(Exports.named("my-attr"))
-          .toInstance(
-              (cd, qp, plugin) -> {
-                try {
-                  String topic = cd.change().getTopic();
-                  if (Strings.isNullOrEmpty(topic)) {
-                    return null;
-                  }
-                  MyInfo myInfo = new MyInfo();
-                  myInfo.theAttribute = "topic: " + topic;
-                  return myInfo;
-                } catch (OrmException e) {
-                  throw new RuntimeException(e);
-                }
-              });
-    }
-  }
-
-  @Test
-  public void queryChangeWithCustomAttributes() throws Exception {
-    Change.Id id1 = createChange().getChange().getId();
-    Change.Id id2 = createChange().getChange().getId();
-    gApi.changes().id(id2.get()).topic("foo");
-
-    Function<Change.Id, QueryRequest> byQuery =
-        id ->
-            gApi.changes().query(id.toString()).withOptions(EnumSet.allOf(ListChangesOption.class));
-    assertThat(gApi.changes().id(id1.get()).get().plugins).isNull();
-    assertThat(gApi.changes().id(id2.get()).get().plugins).isNull();
-    assertThat(Iterables.getOnlyElement(byQuery.apply(id1).get()).plugins).isNull();
-    assertThat(Iterables.getOnlyElement(byQuery.apply(id2).get()).plugins).isNull();
-
-    try (AutoCloseable ignored = installPlugin("my-plugin", CustomAttributeModule.class)) {
-      // Plugin attributes only set in queries.
-      assertThat(gApi.changes().id(id1.get()).get().plugins).isNull();
-      assertThat(gApi.changes().id(id2.get()).get().plugins).isNull();
-
-      assertThat(Iterables.getOnlyElement(byQuery.apply(id1).get()).plugins).isNull();
-      List<PluginDefinedInfo> pluginInfos =
-          Iterables.getOnlyElement(byQuery.apply(id2).get()).plugins;
-      assertThat(pluginInfos).hasSize(1);
-      assertThat(pluginInfos.get(0)).isInstanceOf(CustomAttributeModule.MyInfo.class);
-      CustomAttributeModule.MyInfo myInfo = (CustomAttributeModule.MyInfo) pluginInfos.get(0);
-      assertThat(myInfo.name).isEqualTo("my-plugin");
-      assertThat(myInfo.theAttribute).isEqualTo("topic: foo");
-    }
-
-    assertThat(gApi.changes().id(id1.get()).get().plugins).isNull();
-    assertThat(gApi.changes().id(id2.get()).get().plugins).isNull();
-    assertThat(Iterables.getOnlyElement(byQuery.apply(id1).get()).plugins).isNull();
-    assertThat(Iterables.getOnlyElement(byQuery.apply(id2).get()).plugins).isNull();
-  }
-
   @Test
   public void checkReviewedFlagBeforeAndAfterReview() throws Exception {
     PushOneCommit.Result r = createChange();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
new file mode 100644
index 0000000..86b55bc
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
@@ -0,0 +1,219 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.api.change;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.truth.Truth.assertThat;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.io.CharStreams;
+import com.google.common.reflect.TypeToken;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.query.change.ChangeQueryProcessor.ChangeAttributeFactory;
+import com.google.gerrit.server.query.change.OutputStreamQuery;
+import com.google.gson.Gson;
+import com.google.inject.AbstractModule;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import org.junit.Test;
+
+@UseSsh
+public class PluginFieldsIT extends AbstractDaemonTest {
+  static class MyInfo extends PluginDefinedInfo {
+    @Nullable String theAttribute;
+
+    MyInfo(@Nullable String theAttribute) {
+      this.theAttribute = theAttribute;
+    }
+
+    MyInfo(String name, @Nullable String theAttribute) {
+      this.name = requireNonNull(name);
+      this.theAttribute = theAttribute;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (!(o instanceof MyInfo)) {
+        return false;
+      }
+      MyInfo i = (MyInfo) o;
+      return Objects.equals(name, i.name) && Objects.equals(theAttribute, i.theAttribute);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(name, theAttribute);
+    }
+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this)
+          .add("name", name)
+          .add("theAttribute", theAttribute)
+          .toString();
+    }
+  }
+
+  static class NullAttributeModule extends AbstractModule {
+    @Override
+    public void configure() {
+      bind(ChangeAttributeFactory.class)
+          .annotatedWith(Exports.named("always-null"))
+          .toInstance((cd, qp, p) -> null);
+    }
+  }
+
+  static class SimpleAttributeModule extends AbstractModule {
+    @Override
+    public void configure() {
+      bind(ChangeAttributeFactory.class)
+          .annotatedWith(Exports.named("simple"))
+          .toInstance((cd, qp, p) -> new MyInfo("change " + cd.getId()));
+    }
+  }
+
+  @Test
+  public void queryChangeApiWithNullAttribute() throws Exception {
+    queryChangeWithNullAttribute(
+        id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()));
+  }
+
+  @Test
+  public void queryChangeRestWithNullAttribute() throws Exception {
+    queryChangeWithNullAttribute(
+        id -> pluginInfoFromSingletonListRest(adminRestSession.get(changeQueryUrl(id))));
+  }
+
+  @Test
+  public void queryChangeSshWithNullAttribute() throws Exception {
+    queryChangeWithNullAttribute(
+        id -> pluginInfoFromSingletonListSsh(adminSshSession.exec(changeQueryCmd(id))));
+  }
+
+  private void queryChangeWithNullAttribute(PluginInfoGetter getter) throws Exception {
+    Change.Id id = createChange().getChange().getId();
+    assertThat(getter.call(id)).isNull();
+
+    try (AutoCloseable ignored = installPlugin("my-plugin", NullAttributeModule.class)) {
+      assertThat(getter.call(id)).isNull();
+    }
+
+    assertThat(getter.call(id)).isNull();
+  }
+
+  @Test
+  public void queryChangeApiWithSimpleAttribute() throws Exception {
+    queryChangeWithSimpleAttribute(
+        id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()));
+  }
+
+  @Test
+  public void queryChangeRestWithSimpleAttribute() throws Exception {
+    queryChangeWithSimpleAttribute(
+        id -> pluginInfoFromSingletonListRest(adminRestSession.get(changeQueryUrl(id))));
+  }
+
+  @Test
+  public void queryChangeSshWithSimpleAttribute() throws Exception {
+    queryChangeWithSimpleAttribute(
+        id -> pluginInfoFromSingletonListSsh(adminSshSession.exec(changeQueryCmd(id))));
+  }
+
+  private void queryChangeWithSimpleAttribute(PluginInfoGetter getter) throws Exception {
+    Change.Id id = createChange().getChange().getId();
+    assertThat(getter.call(id)).isNull();
+
+    try (AutoCloseable ignored = installPlugin("my-plugin", SimpleAttributeModule.class)) {
+      List<MyInfo> infos = getter.call(id);
+      assertThat(infos).containsExactly(new MyInfo("my-plugin", "change " + id));
+    }
+
+    assertThat(getter.call(id)).isNull();
+  }
+
+  private String changeQueryUrl(Change.Id id) {
+    return "/changes/?q=" + id;
+  }
+
+  private String changeQueryCmd(Change.Id id) {
+    return "gerrit query --format json " + id;
+  }
+
+  private static List<MyInfo> pluginInfoFromSingletonList(List<ChangeInfo> changeInfos) {
+    assertThat(changeInfos).hasSize(1);
+    List<PluginDefinedInfo> pluginInfo = changeInfos.get(0).plugins;
+    if (pluginInfo == null) {
+      return null;
+    }
+    return pluginInfo.stream().map(MyInfo.class::cast).collect(toImmutableList());
+  }
+
+  @Nullable
+  private static List<MyInfo> pluginInfoFromSingletonListRest(RestResponse res) throws Exception {
+    res.assertOK();
+
+    // Don't deserialize to ChangeInfo directly, since that would treat the plugins field as
+    // List<PluginDefinedInfo> and ignore the unknown keys found in MyInfo.
+    Gson gson = OutputFormat.JSON.newGson();
+    List<Map<String, Object>> changeInfos =
+        gson.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
+    assertThat(changeInfos).hasSize(1);
+    Object plugins = changeInfos.get(0).get("plugins");
+    if (plugins == null) {
+      return null;
+    }
+    return gson.fromJson(gson.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
+  }
+
+  @Nullable
+  private static List<MyInfo> pluginInfoFromSingletonListSsh(String sshOutput) throws Exception {
+    Gson gson = OutputStreamQuery.GSON;
+    List<Map<String, Object>> changeAttrs = new ArrayList<>();
+    for (String line : CharStreams.readLines(new StringReader(sshOutput))) {
+      // Don't deserialize to ChangeAttribute directly, since that would treat the plugins field as
+      // List<PluginDefinedInfo> and ignore the unknown keys found in MyInfo.
+      Map<String, Object> changeAttr =
+          gson.fromJson(line, new TypeToken<Map<String, Object>>() {}.getType());
+      if (!"stats".equals(changeAttr.get("type"))) {
+        changeAttrs.add(changeAttr);
+      }
+    }
+
+    assertThat(changeAttrs).hasSize(1);
+
+    Object plugins = changeAttrs.get(0).get("plugins");
+    if (plugins == null) {
+      return null;
+    }
+    return gson.fromJson(gson.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
+  }
+
+  @FunctionalInterface
+  private interface PluginInfoGetter {
+    List<MyInfo> call(Change.Id id) throws Exception;
+  }
+}