Allow ExternalIncludedIn extensions to add multiple Included In rows

So far ExternalIncludedIn extensions could only add a single row to
the Included In panel. The binding was done via DynamicMap and the
export name was used as row title, while the row values were provided
by the implementation of the ExternalIncludedIn interface. This meant
that multiple bindings were required if multiple rows needed to be
added to the Included In panel. This is bad, if you have an expensive
computation that results in multiple rows, because you want to do the
computation only once and passing the result between different
implementations of the ExternalIncludedIn extension point is
difficult.

Now the ExternalIncludedIn extension point allows to provide multiple
Inlcuded In rows from a single implementation. This is done by
returning the rows as multimap where the key is the row title. The
binding is then done via DynamicSet.

This breaks existing implementations of the ExternalIncludedIn
extension point, but likely this extension point is not used yet (at
least there is no known implementation). Also adaption existing
implementations to the new interface is straight-forward.

Change-Id: I938deed6b1d197dc156c9fe965f0357ff1fe65c3
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-extension-api/BUCK

@@ -27,6 +27,7 @@ java_library(
   name = 'lib',
   exported_deps = [
     ':api',
+    '//lib:guava',
     '//lib/guice:guice',
     '//lib/guice:guice-assistedinject',
     '//lib/guice:guice-servlet',
@@ -42,6 +43,7 @@ java_library(
     '//gerrit-common:annotations',
   ],
   provided_deps = [
+    '//lib:guava',
     '//lib/guice:guice',
     '//lib/guice:guice-assistedinject',
   ],

gerrit-extension-api/BUILD

@@ -23,6 +23,7 @@ java_library(
   name = 'lib',
   exports = [
     ':api',
+    '//lib:guava',
     '//lib/guice:guice',
     '//lib/guice:guice-assistedinject',
     '//lib/guice:guice-servlet',
@@ -37,6 +38,7 @@ java_library(
   srcs = glob([SRC + '**/*.java']),
   deps = [
     '//gerrit-common:annotations',
+    '//lib:guava',
     '//lib/guice:guice',
     '//lib/guice:guice-assistedinject',
   ],

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/config/ExternalIncludedIn.java

@@ -14,16 +14,18 @@
 
 package com.google.gerrit.extensions.config;
 
+import com.google.common.collect.Multimap;
 import com.google.gerrit.extensions.annotations.ExtensionPoint;
 
 import java.util.Collection;
-import java.util.List;
 
 @ExtensionPoint
 public interface ExternalIncludedIn {
 
   /**
-   * Returns a list of systems that include the given commit.
+   * Returns additional entries for IncludedInInfo as multimap where the
+   * key is the row title and the the values are a list of systems that include
+   * the given commit (e.g. names of servers on which this commit is deployed).
    *
    * The tags and branches in which the commit is included are provided so that
    * a RevWalk can be avoided when a system runs a certain tag or branch.
@@ -33,9 +35,8 @@ public interface ExternalIncludedIn {
    *        included
    * @param tags the tags that include the commit
    * @param branches the branches that include the commit
-   * @return a list of systems that contain the given commit, e.g. names of
-   *         servers on which this commit is deployed
+   * @return additional entries for IncludedInInfo
    */
-  List<String> getIncludedIn(String project, String commit,
+  Multimap<String, String> getIncludedIn(String project, String commit,
       Collection<String> tags, Collection<String> branches);
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/IncludedIn.java

@@ -14,8 +14,10 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
 import com.google.gerrit.extensions.config.ExternalIncludedIn;
-import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestReadView;
@@ -39,7 +41,6 @@ import org.eclipse.jgit.revwalk.RevWalk;
 
 import java.io.IOException;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.Map;
 
 @Singleton
@@ -48,13 +49,13 @@ class IncludedIn implements RestReadView<ChangeResource> {
   private final Provider<ReviewDb> db;
   private final GitRepositoryManager repoManager;
   private final PatchSetUtil psUtil;
-  private final DynamicMap<ExternalIncludedIn> includedIn;
+  private final DynamicSet<ExternalIncludedIn> includedIn;
 
   @Inject
   IncludedIn(Provider<ReviewDb> db,
       GitRepositoryManager repoManager,
       PatchSetUtil psUtil,
-      DynamicMap<ExternalIncludedIn> includedIn) {
+      DynamicSet<ExternalIncludedIn> includedIn) {
     this.db = db;
     this.repoManager = repoManager;
     this.psUtil = psUtil;
@@ -80,13 +81,13 @@ class IncludedIn implements RestReadView<ChangeResource> {
       }
 
       IncludedInResolver.Result d = IncludedInResolver.resolve(r, rw, rev);
-      Map<String, Collection<String>> external = new HashMap<>();
-      for (DynamicMap.Entry<ExternalIncludedIn> i : includedIn) {
-        external.put(i.getExportName(),
-            i.getProvider().get().getIncludedIn(
-                project.get(), rev.name(), d.getTags(), d.getBranches()));
+      Multimap<String, String> external = ArrayListMultimap.create();
+      for (ExternalIncludedIn ext : includedIn) {
+        external.putAll(ext.getIncludedIn(project.get(), rev.name(),
+            d.getTags(), d.getBranches()));
       }
-      return new IncludedInInfo(d, (!external.isEmpty() ? external : null));
+      return new IncludedInInfo(d,
+          (!external.isEmpty() ? external.asMap() : null));
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -342,7 +342,7 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicMap.mapOf(binder(), DownloadScheme.class);
     DynamicMap.mapOf(binder(), DownloadCommand.class);
     DynamicMap.mapOf(binder(), CloneCommand.class);
-    DynamicMap.mapOf(binder(), ExternalIncludedIn.class);
+    DynamicSet.setOf(binder(), ExternalIncludedIn.class);
     DynamicMap.mapOf(binder(), ProjectConfigEntry.class);
     DynamicSet.setOf(binder(), PatchSetWebLink.class);
     DynamicSet.setOf(binder(), FileWebLink.class);