Defer loading site libraries until database opens

By deferring the library loading a new MySQL database driver can
be downloaded and the old one removed from the lib/ directory before
it is added to the current URLClassLoader. This avoids seeing the
library twice in the classpath.

Bug: issue 1870
Change-Id: I5600d6cc998aad1c702058512b3f7371d547a95e

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Init.java

@@ -21,7 +21,6 @@ import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.pgm.init.Browser;
 import com.google.gerrit.pgm.init.InitFlags;
 import com.google.gerrit.pgm.init.InitModule;
-import com.google.gerrit.pgm.init.ReloadSiteLibrary;
 import com.google.gerrit.pgm.init.SitePathInitializer;
 import com.google.gerrit.pgm.util.ConsoleUI;
 import com.google.gerrit.pgm.util.Die;
@@ -121,12 +120,6 @@ public class Init extends SiteProgram {
       protected void configure() {
         bind(ConsoleUI.class).toInstance(ui);
         bind(File.class).annotatedWith(SitePath.class).toInstance(sitePath);
-        bind(ReloadSiteLibrary.class).toInstance(new ReloadSiteLibrary() {
-          @Override
-          public void reload() {
-            Init.super.loadSiteLib();
-          }
-        });
       }
     });
 

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/LibraryDownloader.java

@@ -17,6 +17,7 @@ package com.google.gerrit.pgm.init;
 import com.google.common.base.Strings;
 import com.google.gerrit.pgm.util.ConsoleUI;
 import com.google.gerrit.pgm.util.Die;
+import com.google.gerrit.pgm.util.IoUtil;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.inject.Inject;
 
@@ -42,7 +43,6 @@ import java.security.NoSuchAlgorithmException;
 class LibraryDownloader {
   private final ConsoleUI ui;
   private final File lib_dir;
-  private final ReloadSiteLibrary reload;
 
   private boolean required;
   private String name;
@@ -52,11 +52,9 @@ class LibraryDownloader {
   private File dst;
 
   @Inject
-  LibraryDownloader(final ReloadSiteLibrary reload, final ConsoleUI ui,
+  LibraryDownloader(ConsoleUI ui, SitePaths site) {
-      final SitePaths site) {
     this.ui = ui;
     this.lib_dir = site.lib_dir;
-    this.reload = reload;
   }
 
   void setName(final String name) {
@@ -163,7 +161,9 @@ class LibraryDownloader {
       }
     }
 
-    reload.reload();
+    if (dst.exists()) {
+      IoUtil.loadJARs(dst);
+    }
   }
 
   private void removeStaleVersions() {

gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/ReloadSiteLibrary.java

@@ -1,20 +0,0 @@
-// Copyright (C) 2009 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.pgm.init;
-
-/** Requests the site's {@code lib/} directory be scanned again. */
-public interface ReloadSiteLibrary {
-  public void reload();
-}

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/IoUtil.java

@@ -14,9 +14,19 @@
 
 package com.google.gerrit.pgm.util;
 
+import com.google.common.collect.Sets;
+
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.Arrays;
+import java.util.Set;
 
 public final class IoUtil {
   public static void copyWithThread(final InputStream src,
@@ -42,6 +52,47 @@ public final class IoUtil {
     }.start();
   }
 
+  public static void loadJARs(File... jars) {
+    ClassLoader cl = IoUtil.class.getClassLoader();
+    if (!(cl instanceof URLClassLoader)) {
+      throw noAddURL("Not loaded by URLClassLoader", null);
+    }
+    @SuppressWarnings("resource")
+    URLClassLoader urlClassLoader = (URLClassLoader) cl;
+
+    Method addURL;
+    try {
+      addURL = URLClassLoader.class.getDeclaredMethod("addURL", URL.class);
+      addURL.setAccessible(true);
+    } catch (SecurityException e) {
+      throw noAddURL("Method addURL not available", e);
+    } catch (NoSuchMethodException e) {
+      throw noAddURL("Method addURL not available", e);
+    }
+
+    Set<URL> have = Sets.newHashSet(Arrays.asList(urlClassLoader.getURLs()));
+    for (File path : jars) {
+      try {
+        URL url = path.toURI().toURL();
+        if (have.add(url)) {
+          addURL.invoke(cl, url);
+        }
+      } catch (MalformedURLException e) {
+        throw noAddURL("addURL " + path + " failed", e);
+      } catch (IllegalArgumentException e) {
+        throw noAddURL("addURL " + path + " failed", e);
+      } catch (IllegalAccessException e) {
+        throw noAddURL("addURL " + path + " failed", e);
+      } catch (InvocationTargetException e) {
+        throw noAddURL("addURL " + path + " failed", e.getCause());
+      }
+    }
+  }
+
+  private static UnsupportedOperationException noAddURL(String m, Throwable why) {
+    String prefix = "Cannot extend classpath: ";
+    return new UnsupportedOperationException(prefix + m, why);
+  }
   private IoUtil() {
   }
 }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteLibraryBasedDataSourceProvider.java

@@ -0,0 +1,75 @@
+// Copyright (C) 2013 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.pgm.util;
+
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.schema.DataSourceProvider;
+import com.google.gerrit.server.schema.DataSourceType;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.lib.Config;
+
+import java.io.File;
+import java.io.FileFilter;
+import java.util.Arrays;
+import java.util.Comparator;
+
+import javax.sql.DataSource;
+
+/** Loads the site library if not yet loaded. */
+@Singleton
+public class SiteLibraryBasedDataSourceProvider extends DataSourceProvider {
+  private final File libdir;
+  private boolean init;
+
+  @Inject
+  SiteLibraryBasedDataSourceProvider(SitePaths site,
+      @GerritServerConfig Config cfg,
+      DataSourceProvider.Context ctx,
+      DataSourceType dst) {
+    super(site, cfg, ctx, dst);
+    libdir = site.lib_dir;
+  }
+
+  public synchronized DataSource get() {
+    if (!init) {
+      loadSiteLib();
+      init = true;
+    }
+    return super.get();
+  }
+
+  private void loadSiteLib() {
+    File[] jars = libdir.listFiles(new FileFilter() {
+      @Override
+      public boolean accept(File path) {
+        String name = path.getName();
+        return (name.endsWith(".jar") || name.endsWith(".zip"))
+            && path.isFile();
+      }
+    });
+    if (jars != null && 0 < jars.length) {
+      Arrays.sort(jars, new Comparator<File>() {
+        @Override
+        public int compare(File a, File b) {
+          return a.getName().compareTo(b.getName());
+        }
+      });
+      IoUtil.loadJARs(jars);
+    }
+  }
+}

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java

@@ -41,19 +41,9 @@ import org.eclipse.jgit.lib.Config;
 import org.kohsuke.args4j.Option;
 
 import java.io.File;
-import java.io.FileFilter;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.net.URLClassLoader;
 import java.sql.SQLException;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Comparator;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import javax.sql.DataSource;
 
@@ -78,77 +68,8 @@ public abstract class SiteProgram extends AbstractProgram {
     }
   }
 
-  /** Load extra JARs from {@code lib/} subdirectory of {@link #getSitePath()} */
-  protected void loadSiteLib() {
-    final File libdir = new File(getSitePath(), "lib");
-    final File[] list = libdir.listFiles(new FileFilter() {
-      @Override
-      public boolean accept(File path) {
-        if (!path.isFile()) {
-          return false;
-        }
-        return path.getName().endsWith(".jar") //
-            || path.getName().endsWith(".zip");
-      }
-    });
-    if (list != null && 0 < list.length) {
-      Arrays.sort(list, new Comparator<File>() {
-        @Override
-        public int compare(File a, File b) {
-          return a.getName().compareTo(b.getName());
-        }
-      });
-      addToClassLoader(list);
-    }
-  }
-
-  private void addToClassLoader(final File[] additionalLocations) {
-    final ClassLoader cl = getClass().getClassLoader();
-    if (!(cl instanceof URLClassLoader)) {
-      throw noAddURL("Not loaded by URLClassLoader", null);
-    }
-
-    final URLClassLoader ucl = (URLClassLoader) cl;
-    final Set<URL> have = new HashSet<URL>();
-    have.addAll(Arrays.asList(ucl.getURLs()));
-
-    final Method m;
-    try {
-      m = URLClassLoader.class.getDeclaredMethod("addURL", URL.class);
-      m.setAccessible(true);
-    } catch (SecurityException e) {
-      throw noAddURL("Method addURL not available", e);
-    } catch (NoSuchMethodException e) {
-      throw noAddURL("Method addURL not available", e);
-    }
-
-    for (final File path : additionalLocations) {
-      try {
-        final URL url = path.toURI().toURL();
-        if (have.add(url)) {
-          m.invoke(cl, url);
-        }
-      } catch (MalformedURLException e) {
-        throw noAddURL("addURL " + path + " failed", e);
-      } catch (IllegalArgumentException e) {
-        throw noAddURL("addURL " + path + " failed", e);
-      } catch (IllegalAccessException e) {
-        throw noAddURL("addURL " + path + " failed", e);
-      } catch (InvocationTargetException e) {
-        throw noAddURL("addURL " + path + " failed", e.getCause());
-      }
-    }
-  }
-
-  private static UnsupportedOperationException noAddURL(String m, Throwable why) {
-    final String prefix = "Cannot extend classpath: ";
-    return new UnsupportedOperationException(prefix + m, why);
-  }
-
   /** @return provides database connectivity and site path. */
   protected Injector createDbInjector(final DataSourceProvider.Context context) {
-    loadSiteLib();
-
     final File sitePath = getSitePath();
     final List<Module> modules = new ArrayList<Module>();
 
@@ -164,9 +85,10 @@ public abstract class SiteProgram extends AbstractProgram {
       @Override
       protected void configure() {
         bind(DataSourceProvider.Context.class).toInstance(context);
-        bind(Key.get(DataSource.class, Names.named("ReviewDb"))).toProvider(
+        bind(Key.get(DataSource.class, Names.named("ReviewDb")))
-            DataSourceProvider.class).in(SINGLETON);
+          .toProvider(SiteLibraryBasedDataSourceProvider.class)
-        listener().to(DataSourceProvider.class);
+          .in(SINGLETON);
+        listener().to(SiteLibraryBasedDataSourceProvider.class);
       }
     });
     Module configModule = new GerritServerConfigModule();

gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/LibrariesTest.java

@@ -30,16 +30,14 @@ import java.io.FileNotFoundException;
 public class LibrariesTest extends TestCase {
   public void testCreate() throws FileNotFoundException {
     final SitePaths site = new SitePaths(new File("."));
-    final ReloadSiteLibrary reload = createStrictMock(ReloadSiteLibrary.class);
     final ConsoleUI ui = createStrictMock(ConsoleUI.class);
 
     replay(ui);
-    replay(reload);
 
     Libraries lib = new Libraries(new Provider<LibraryDownloader>() {
       @Override
       public LibraryDownloader get() {
-        return new LibraryDownloader(reload, ui, site);
+        return new LibraryDownloader(ui, site);
       }
     });
 
@@ -47,6 +45,5 @@ public class LibrariesTest extends TestCase {
     assertNotNull(lib.mysqlDriver);
 
     verify(ui);
-    verify(reload);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/schema/DataSourceProvider.java

@@ -39,18 +39,30 @@ import javax.sql.DataSource;
 
 /** Provides access to the DataSource. */
 @Singleton
-public final class DataSourceProvider implements Provider<DataSource>,
+public class DataSourceProvider implements Provider<DataSource>,
     LifecycleListener {
-  private final DataSource ds;
+  private final SitePaths site;
+  private final Config cfg;
+  private final Context ctx;
+  private final DataSourceType dst;
+  private DataSource ds;
 
   @Inject
-  DataSourceProvider(final SitePaths site,
+  protected DataSourceProvider(SitePaths site,
-      @GerritServerConfig final Config cfg, Context ctx, DataSourceType dst) {
+      @GerritServerConfig Config cfg,
-    ds = open(site, cfg, ctx, dst);
+      Context ctx,
+      DataSourceType dst) {
+    this.site = site;
+    this.cfg = cfg;
+    this.ctx = ctx;
+    this.dst = dst;
   }
 
   @Override
   public synchronized DataSource get() {
+    if (ds == null) {
+      ds = open(site, cfg, ctx, dst);
+    }
     return ds;
   }