Push plugin Guice Module load into the ServerPlugin loader

The responsibility of parsing the Guice Modules list from
Manifest file should be common to all ServerPlugins and
located into the ServerPlugin class.

This allows to avoid duplicates in the parsing and interpretation
of plugin Manifest for all other subclasses of ServerPlugins
provided by other external plugins providers.

Change-Id: If0c8c502517adc53ac30569d6c52e357d3c9f7dd

gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java

@@ -18,7 +18,6 @@ import com.google.common.base.CharMatcher;
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
 import com.google.common.base.Predicate;
-import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
@@ -38,7 +37,6 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.plugins.ServerPluginProvider.PluginDescription;
 import com.google.inject.Inject;
-import com.google.inject.Module;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
@@ -72,7 +70,6 @@ import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
-import java.util.jar.Attributes;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
 
@@ -592,16 +589,6 @@ public class PluginLoader implements LifecycleListener {
     try {
       Manifest manifest = jarFile.getManifest();
       Plugin.ApiType type = Plugin.getApiType(manifest);
-      Attributes main = manifest.getMainAttributes();
-      String sysName = main.getValue("Gerrit-Module");
-      String sshName = main.getValue("Gerrit-SshModule");
-      String httpName = main.getValue("Gerrit-HttpModule");
-
-      if (!Strings.isNullOrEmpty(sshName) && type != Plugin.ApiType.PLUGIN) {
-        throw new InvalidPluginException(String.format(
-            "Using Gerrit-SshModule requires Gerrit-ApiType: %s",
-            Plugin.ApiType.PLUGIN));
-      }
 
       List<URL> urls = new ArrayList<>(2);
       String overlay = System.getProperty("gerrit.plugin-classes");
@@ -619,16 +606,12 @@ public class PluginLoader implements LifecycleListener {
       ClassLoader pluginLoader = new URLClassLoader(
           urls.toArray(new URL[urls.size()]),
           parentFor(type));
-      Class<? extends Module> sysModule = load(sysName, pluginLoader);
-      Class<? extends Module> sshModule = load(sshName, pluginLoader);
-      Class<? extends Module> httpModule = load(httpName, pluginLoader);
 
       Plugin plugin = new ServerPlugin(name, getPluginCanonicalWebUrl(name),
           pluginUserFactory.create(name),
           srcJar, snapshot,
           new JarScanner(srcJar),
-          getPluginDataDir(name), type, pluginLoader,
+          getPluginDataDir(name), pluginLoader);
-          sysModule, sshModule, httpModule);
       cleanupHandles.put(plugin, new CleanupHandle(tmp, jarFile));
       keep = true;
       return plugin;
@@ -681,23 +664,6 @@ public class PluginLoader implements LifecycleListener {
     return PLUGIN_TMP_PREFIX + name + "_" + fmt.format(new Date()) + "_";
   }
 
-  private static Class<? extends Module> load(String name, ClassLoader pluginLoader)
-      throws ClassNotFoundException {
-    if (Strings.isNullOrEmpty(name)) {
-      return null;
-    }
-
-    @SuppressWarnings("unchecked")
-    Class<? extends Module> clazz =
-        (Class<? extends Module>) Class.forName(name, false, pluginLoader);
-    if (!Module.class.isAssignableFrom(clazz)) {
-      throw new ClassCastException(String.format(
-          "Class %s does not implement %s",
-          name, Module.class.getName()));
-    }
-    return clazz;
-  }
-
   // Only one active plugin per plugin name can exist for each plugin name.
   // Filter out disabled plugins and transform the multimap to a map
   private static Map<String, File> filterDisabled(

gerrit-server/src/main/java/com/google/gerrit/server/plugins/ServerPlugin.java

@@ -79,28 +79,59 @@ public class ServerPlugin extends Plugin {
       FileSnapshot snapshot,
       PluginContentScanner scanner,
       File dataDir,
-      ApiType apiType,
+      ClassLoader classLoader) throws InvalidPluginException {
-      ClassLoader classLoader,
+    super(name, srcJar, pluginUser, snapshot, Plugin.getApiType(getPluginManifest(scanner)));
-      @Nullable Class<? extends Module> sysModule,
-      @Nullable Class<? extends Module> sshModule,
-      @Nullable Class<? extends Module> httpModule)
-      throws InvalidPluginException {
-    super(name, srcJar, pluginUser, snapshot, apiType);
     this.pluginCanonicalWebUrl = pluginCanonicalWebUrl;
     this.scanner = scanner;
     this.dataDir = dataDir;
     this.classLoader = classLoader;
-    this.sysModule = sysModule;
-    this.sshModule = sshModule;
-    this.httpModule = httpModule;
     this.manifest = getPluginManifest(scanner);
+    loadGuiceModules(manifest, classLoader);
+  }
+
+  private void loadGuiceModules(Manifest manifest, ClassLoader classLoader) throws InvalidPluginException {
+    Attributes main = manifest.getMainAttributes();
+    String sysName = main.getValue("Gerrit-Module");
+    String sshName = main.getValue("Gerrit-SshModule");
+    String httpName = main.getValue("Gerrit-HttpModule");
+
+    if (!Strings.isNullOrEmpty(sshName) && getApiType() != Plugin.ApiType.PLUGIN) {
+      throw new InvalidPluginException(String.format(
+          "Using Gerrit-SshModule requires Gerrit-ApiType: %s",
+          Plugin.ApiType.PLUGIN));
+    }
+
+    try {
+      this.sysModule = load(sysName, classLoader);
+      this.sshModule = load(sshName, classLoader);
+      this.httpModule = load(httpName, classLoader);
+    } catch(ClassNotFoundException e) {
+      throw new InvalidPluginException("Unable to load plugin Guice Modules", e);
+    }
+  }
+
+  @SuppressWarnings("unchecked")
+  private static Class<? extends Module> load(String name, ClassLoader pluginLoader)
+      throws ClassNotFoundException {
+    if (Strings.isNullOrEmpty(name)) {
+      return null;
+    }
+
+    Class<?> clazz =
+        Class.forName(name, false, pluginLoader);
+    if (!Module.class.isAssignableFrom(clazz)) {
+      throw new ClassCastException(String.format(
+          "Class %s does not implement %s",
+          name, Module.class.getName()));
+    }
+    return (Class<? extends Module>) clazz;
   }
 
   File getSrcJar() {
     return getSrcFile();
   }
 
-  private Manifest getPluginManifest(PluginContentScanner scanner)
+  private static Manifest getPluginManifest(PluginContentScanner scanner)
       throws InvalidPluginException {
     try {
        return scanner.getManifest();