Add support for --depends-on--all query option Add --depends-on--all option to gerrit query SSH command and REST API. This option will add a plugin section to the output which displays all the depends-ons from the latest Depends-on comment on the change. This option helps in building a depends-on web UI in future. Also add functional tests and documentation. Change-Id: I713f7765f47d115d499299f449fe08a70dd1a532

diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java
index 5bfccd5..6901c90 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java

@@ -85,7 +85,11 @@
    *
    * <p>return empty set means no dependencies found.
    */
-  public Set<DependsOn> load(Change.Id cid) {
+  public Set<DependsOn> load(Change.Id cid) throws StorageException {
+    return loadWithOrder(cid).stream().collect(Collectors.toSet());
+  }
+
+  public List<DependsOn> loadWithOrder(Change.Id cid) throws StorageException {
     ChangeNotes changeNote = changeNotesFactory.createCheckedUsingIndexLookup(cid);
     List<ChangeMessage> messages = cmUtil.byChange(changeNote);
     List<ChangeMessage> sortedChangeMessages =
@@ -93,12 +97,12 @@
             .sorted(Comparator.comparing(ChangeMessage::getWrittenOn).reversed())
             .collect(Collectors.toCollection(ArrayList::new));
     for (ChangeMessage message : sortedChangeMessages) {
-      Optional<Set<DependsOn>> deps = Comment.from(message.getMessage());
+      Optional<List<DependsOn>> deps = Comment.from(message.getMessage());
       if (deps.isPresent()) {
         return deps.get();
       }
     }
-    return Collections.emptySet();
+    return Collections.emptyList();
   }
 
   /** If needed, create a comment on the change with a DependsOn for the dependencies. */

diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnAttributeFactory.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnAttributeFactory.java
new file mode 100644
index 0000000..3badb76
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnAttributeFactory.java

@@ -0,0 +1,92 @@
+// Copyright (C) 2022 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.googlesource.gerrit.plugins.depends.on;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.server.DynamicOptions;
+import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
+import com.google.gerrit.server.query.change.ChangeData;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DependsOnAttributeFactory implements ChangePluginDefinedInfoFactory {
+  private static final Logger log = LoggerFactory.getLogger(DependsOnAttributeFactory.class);
+  protected static final String FAILED_TO_LOAD_DEPENDS_ON = "Failed to load Depends-on";
+
+  protected final ChangeMessageStore changeMessageStore;
+
+  @Inject
+  DependsOnAttributeFactory(ChangeMessageStore changeMessageStore) {
+    this.changeMessageStore = changeMessageStore;
+  }
+
+  @Override
+  public Map<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+      Collection<ChangeData> cds, DynamicOptions.BeanProvider beanProvider, String plugin) {
+    if (!((Module.MyQueryOptions) beanProvider.getDynamicBean(plugin)).all) {
+      return Collections.emptyMap();
+    }
+    Map<Change.Id, PluginDefinedInfo> dependsOnAttributesByChange = new HashMap<>();
+    for (ChangeData changeData : cds) {
+      try {
+        List<DependsOn> dependsOns = changeMessageStore.loadWithOrder(changeData.getId());
+        if (dependsOns.size() > 0) {
+          DependsOnPluginAttributes dependsOnPluginAttributes = new DependsOnPluginAttributes();
+          dependsOnPluginAttributes.addDependsOns(dependsOns);
+          dependsOnAttributesByChange.put(changeData.getId(), dependsOnPluginAttributes);
+        }
+      } catch (StorageException e) {
+        log.error(FAILED_TO_LOAD_DEPENDS_ON, e);
+        PluginDefinedInfo pluginDefinedInfo = new PluginDefinedInfo();
+        pluginDefinedInfo.message = FAILED_TO_LOAD_DEPENDS_ON;
+        dependsOnAttributesByChange.put(changeData.getId(), pluginDefinedInfo);
+      }
+    }
+    return dependsOnAttributesByChange;
+  }
+
+  protected static class DependsOnPluginAttributes extends PluginDefinedInfo {
+    public List<DependsOnAttribute> dependsOns = new ArrayList<>();
+
+    public void addDependsOns(List<DependsOn> dependsOns) {
+      for (DependsOn dependsOn : dependsOns) {
+        this.dependsOns.add(new DependsOnAttribute(dependsOn));
+      }
+    }
+  }
+
+  protected static class DependsOnAttribute {
+    @Nullable protected Integer changeNumber;
+    @Nullable protected String unresolved;
+
+    public DependsOnAttribute(DependsOn dependsOn) {
+      if (dependsOn.isResolved()) {
+        changeNumber = dependsOn.id().get();
+      } else {
+        unresolved = dependsOn.key().get();
+      }
+    }
+  }
+}

diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/Module.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/Module.java
index 21644c1..b6eb1e1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/Module.java

@@ -14,16 +14,27 @@
 
 package com.googlesource.gerrit.plugins.depends.on;
 
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.annotations.Exports;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.validators.CommentValidator;
 import com.google.gerrit.server.DynamicOptions.DynamicBean;
+import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
 import com.google.gerrit.server.events.EventListener;
+import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.query.change.ChangeQueryBuilder.ChangeOperatorFactory;
 import com.google.gerrit.server.restapi.change.GetChange;
 import com.google.gerrit.server.restapi.change.QueryChanges;
 import com.google.gerrit.sshd.commands.Query;
 import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.depends.on.extensions.DependencyResolver;
+import java.util.Set;
+import org.kohsuke.args4j.Option;
 
 public class Module extends AbstractModule {
   @Override
@@ -32,15 +43,39 @@
         .annotatedWith(Exports.named(InDependsOnOperator.FIELD))
         .to(InDependsOnOperator.class);
     DynamicSet.bind(binder(), EventListener.class).to(CoreListener.class);
-    bind(DynamicBean.class)
-        .annotatedWith(Exports.named(GetChange.class))
-        .to(ChangeMessageStore.class);
-    bind(DynamicBean.class).annotatedWith(Exports.named(Query.class)).to(ChangeMessageStore.class);
+    bind(ChangePluginDefinedInfoFactory.class)
+        .annotatedWith(Exports.named("depends-ons"))
+        .to(DependsOnAttributeFactory.class);
+    bind(DynamicBean.class).annotatedWith(Exports.named(GetChange.class)).to(MyQueryOptions.class);
+    bind(DynamicBean.class).annotatedWith(Exports.named(Query.class)).to(MyQueryOptions.class);
     bind(DynamicBean.class)
         .annotatedWith(Exports.named(QueryChanges.class))
-        .to(ChangeMessageStore.class);
+        .to(MyQueryOptions.class);
     bind(CommentValidator.class)
         .annotatedWith(Exports.named(DependsOnCommentValidator.class.getSimpleName()))
         .to(DependsOnCommentValidator.class);
   }
+
+  public static class MyQueryOptions implements DependencyResolver {
+    @Option(name = "--all", usage = "Include all depends-on in the output")
+    public boolean all = false;
+
+    protected final ChangeMessageStore changeMessageStore;
+
+    @Inject
+    public MyQueryOptions(ChangeMessageStore changeMessageStore) {
+      this.changeMessageStore = changeMessageStore;
+    }
+
+    @Override
+    public boolean resolveDependencies(PatchSet.Id patchSetId, Set<Set<BranchNameKey>> deliverables)
+        throws InvalidChangeOperationException, StorageException, NoSuchChangeException {
+      return changeMessageStore.resolveDependencies(patchSetId, deliverables);
+    }
+
+    @Override
+    public boolean hasUnresolvedDependsOn(Change.Id changeId) throws StorageException {
+      return changeMessageStore.hasUnresolvedDependsOn(changeId);
+    }
+  }
 }

diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java
index e911560..8b35bcd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java

@@ -16,6 +16,7 @@
 
 import com.googlesource.gerrit.plugins.depends.on.DependsOn;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Matcher;
@@ -30,7 +31,7 @@
   protected static final Pattern DEPENDS_ON_PATTERN = Pattern.compile("^Depends-on:(.*)$");
 
   /** return empty Optional instance means no dependencies found */
-  public static Optional<Set<DependsOn>> from(String comment) {
+  public static Optional<List<DependsOn>> from(String comment) {
     for (String line : comment.split("\n", -1)) {
       Matcher match = DEPENDS_ON_PATTERN.matcher(line);
       if (match.find()) {
@@ -41,7 +42,7 @@
             Arrays.stream(changes.split("\\s+", -1)) // -> ["1234", "4444"]
                 .filter(c -> !c.isEmpty())
                 .map(c -> DependsOn.create(c))
-                .collect(Collectors.toSet()));
+                .collect(Collectors.toList()));
       }
     }
     return Optional.empty();

diff --git a/src/main/resources/Documentation/change-search-attributes.md b/src/main/resources/Documentation/change-search-attributes.md
new file mode 100644
index 0000000..cd8e7cf
--- /dev/null
+++ b/src/main/resources/Documentation/change-search-attributes.md

@@ -0,0 +1,45 @@
+Search Attributes
+=================
+
+Change Query Output
+-------------------
+It is possible to add a dependsOns section to the query output of changes
+using the @PLUGIN@ plugin switches. The following switches are available:
+
+**\-\-@PLUGIN@\-\-all**
+
+This switch is meant to be used to show all depends-ons from the latest
+Depends-on comment on the change. The switch output order matches the
+order from the change comment.
+
+When a change has one or more depends-on associated with it, query output will
+have a "dependsOns" section under the plugins section like below:
+
+```
+  $ ssh -x -p 29418 example.com gerrit query --@PLUGIN@--all \
+        --format JSON change:144193 | head -1 | json_pp
+  {
+     "id" : "Ifc577c2660c26220c39df57627ca1053c3f2067c",
+     ...
+     "plugins" : [
+        {
+           "name" : "@PLUGIN@",
+           "dependsOns" : [
+              {
+                "changeNumber": 732
+              },
+              {
+                "changeNumber": 733
+              },
+              {
+                "unresolved": "Ieace383c14de79bf202c85063d5a46a0580724dd"
+              },
+              {
+                "changeNumber": 734
+              }
+           ],
+           "name": "depends-on"
+        }
+     ]
+  }
+```

diff --git a/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java b/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java
index 52de2a7..491d572 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java

@@ -16,8 +16,8 @@
 
 import com.google.gerrit.testing.InMemoryModule;
 import com.googlesource.gerrit.plugins.depends.on.formats.Comment;
+import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 import junit.framework.TestCase;
 import org.junit.Test;
 
@@ -68,21 +68,21 @@
   @Test
   public void testParseNoneComment() {
     String comment = "My Very Educated Mother Just Served Us Nothing!";
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertFalse(deps.isPresent());
   }
 
   @Test
   public void testParseEmptyComment() {
     String comment = "Depends-on:";
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 0);
   }
 
   @Test
   public void testParseOneNumComment() {
     String comment = "Depends-on:" + NUM;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     for (DependsOn dep : deps.get()) {
       assertTrue(NUM.equals("" + dep.id().get()));
       return;
@@ -93,7 +93,7 @@
   @Test
   public void testParseOneKeyComment() {
     String comment = "Depends-on:" + KEY;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     for (DependsOn dep : deps.get()) {
       assertTrue(KEY.equals("" + dep.key().get()));
       return;
@@ -104,7 +104,7 @@
   @Test
   public void testParseTwoNumsComment() {
     String comment = "Depends-on:" + NUM + " " + NUM2;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 2);
     int found = 0;
     for (DependsOn dep : deps.get()) {
@@ -117,7 +117,7 @@
   @Test
   public void testParseTwoKeyComments() {
     String comment = "Depends-on:" + KEY + " " + KEY2;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 2);
     int found = 0;
     for (DependsOn dep : deps.get()) {
@@ -130,7 +130,7 @@
   @Test
   public void testParseNumAndKeyComment() {
     String comment = "Depends-on:" + NUM + " " + KEY;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 2);
     int found = 0;
     for (DependsOn dep : deps.get()) {
@@ -146,7 +146,7 @@
 
   public void testParseTwoNumsCommaComment() {
     String comment = "Depends-on:" + NUM + "," + NUM2;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 2);
     int found = 0;
     for (DependsOn dep : deps.get()) {
@@ -158,7 +158,7 @@
 
   public void testParseTwoNumsCommaSpaceComment() {
     String comment = "Depends-on:" + NUM + ", " + NUM2;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 2);
     int found = 0;
     for (DependsOn dep : deps.get()) {
@@ -170,7 +170,7 @@
 
   public void testParseTwoNumsWhiteComment() {
     String comment = "Depends-on:" + NUM + ", \t" + NUM2;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 2);
     int found = 0;
     for (DependsOn dep : deps.get()) {
@@ -183,7 +183,7 @@
   public void testParseTwoNumsNewLineWhiteComment() {
     // Should stop processing at newline
     String comment = "Depends-on:" + NUM + ", \t\n" + NUM2;
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 1);
     for (DependsOn dep : deps.get()) {
       assertTrue(NUM.equals("" + dep.id().get()));
@@ -192,7 +192,7 @@
 
   public void testParseEmbeddedComment() {
     String comment = "Patch Set 2:\n\nDepends-on:" + NUM + " " + NUM2 + "\nHey\n";
-    Optional<Set<DependsOn>> deps = Comment.from(comment);
+    Optional<List<DependsOn>> deps = Comment.from(comment);
     assertTrue(deps.get().size() == 2);
     int found = 0;
     for (DependsOn dep : deps.get()) {

diff --git a/test/test_dependson.sh b/test/test_dependson.sh
index cc68ced..7e78684 100755
--- a/test/test_dependson.sh
+++ b/test/test_dependson.sh

@@ -171,7 +171,7 @@
 GIT_DIR=$REPO_DIR/.git
 FILE_A=$REPO_DIR/fileA
 
-# ------------------------- Depends-on Test ---------------------------
+# ------------------------- Depends-on propagation Test ---------------------------
 base_change=$(create_change "$SRC_REF_BRANCH" "$FILE_A") || exit
 src_change=$(create_change "$SRC_REF_BRANCH" "$FILE_A") || exit
 gssh gerrit review --message \'"Depends-on: $base_change"\' "$src_change",1
@@ -180,7 +180,6 @@
 actual=$(get_depends_on_tag "$dest_change")
 result_out "propagate depends-on" "$expected" "$actual"
 
-
 # ------------------------- Depends-on comment validator test ---------------------------
 change=$(create_change "$SRC_REF_BRANCH" "$FILE_A") || exit
 commit=$(mygit log -1 --pretty=format:"%H")
@@ -213,4 +212,19 @@
         '{"comments": {"/PATCHSET_LEVEL":[{"message": "Depends-on: 10 30"}]}}')
 result_out "depends-on patchset level comment - REST" "$expected" "$out"
 
+# ------------------------- Depends-on query Test ---------------------------
+change=$(create_change "$SRC_REF_BRANCH" "$FILE_A") || exit
+gssh gerrit review --message \
+    \'"Depends-on: 10 30 Ieace383c14de79bf202c85063d5a46a0580724dd 20"\' "$change",1
+out=$(query "$change" --depends-on--all | jq --raw-output '.plugins[0].dependsOns')
+result "depends-on query"
+result_out "depends-on query output 1" "10" "$(echo "$out" | jq '.[0].changeNumber')"
+result_out "depends-on query output 2" "30" "$(echo "$out" | jq '.[1].changeNumber')"
+result_out "depends-on query output 3" "Ieace383c14de79bf202c85063d5a46a0580724dd" \
+    "$(echo "$out" | jq --raw-output '.[2].unresolved')"
+result_out "depends-on query output 4" "20" "$(echo "$out" | jq '.[3].changeNumber')"
+result_out "depends-on query output datatype" "number number string number" \
+    "$(echo "$out" | jq --raw-output 'map(select(.changeNumber!=null).changeNumber,
+        select(.unresolved!=null).unresolved | type) | join(" ")')"
+
 exit $RESULT