prevent using same classloader multiple times

scm-webapp/src/main/java/sonia/scm/plugin/UberClassLoader.java

@@ -1,9 +1,9 @@
 /**
  * Copyright (c) 2010, Sebastian Sdorra All rights reserved.
- *
+ * <p>
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
- *
+ * <p>
  * 1. Redistributions of source code must retain the above copyright notice,
  * this list of conditions and the following disclaimer. 2. Redistributions in
  * binary form must reproduce the above copyright notice, this list of
@@ -11,7 +11,7 @@
  * materials provided with the distribution. 3. Neither the name of SCM-Manager;
  * nor the names of its contributors may be used to endorse or promote products
  * derived from this software without specific prior written permission.
- *
+ * <p>
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
  * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
@@ -22,60 +22,59 @@
  * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
  * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
+ * <p>
  * http://bitbucket.org/sdorra/scm-manager
- *
  */
 
 
-
 package sonia.scm.plugin;
 
 //~--- non-JDK imports --------------------------------------------------------
 
-import com.google.common.collect.Lists;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import java.io.IOException;
-
 import java.lang.ref.WeakReference;
-
 import java.net.URL;
-
 import java.util.Collections;
 import java.util.Enumeration;
-import java.util.List;
+import java.util.LinkedHashSet;
+import java.util.Set;
 import java.util.concurrent.ConcurrentMap;
 
+//~--- JDK imports ------------------------------------------------------------
+
 /**
  * {@link ClassLoader} which is able to load classes and resources from all
  * plugins.
  *
  * @author Sebastian Sdorra
  */
-public final class UberClassLoader extends ClassLoader
+public final class UberClassLoader extends ClassLoader {
-{
 
-  /**
+  private final Set<ClassLoader> pluginClassLoaders;
-   * Constructs ...
+  private final ConcurrentMap<String, WeakReference<Class<?>>> cache = Maps.newConcurrentMap();
-   *
+
-   *
+  public UberClassLoader(ClassLoader parent, Iterable<InstalledPlugin> plugins) {
-   * @param parent
+    this(parent, collectClassLoaders(plugins));
-   * @param plugins
-   */
-  public UberClassLoader(ClassLoader parent, Iterable<InstalledPlugin> plugins)
-  {
-    super(parent);
-    this.plugins = plugins;
   }
 
-  //~--- methods --------------------------------------------------------------
+  private static Set<ClassLoader> collectClassLoaders(Iterable<InstalledPlugin> plugins) {
+    ImmutableSet.Builder<ClassLoader> classLoaders = ImmutableSet.builder();
+    plugins.forEach(plugin -> classLoaders.add(plugin.getClassLoader()));
+    return classLoaders.build();
+  }
+
+  @VisibleForTesting
+  UberClassLoader(ClassLoader parent, Set<ClassLoader> pluginClassLoaders) {
+    super(parent);
+    this.pluginClassLoaders = pluginClassLoaders;
+  }
 
   @Override
-  protected Class<?> findClass(String name) throws ClassNotFoundException
+  protected Class<?> findClass(String name) throws ClassNotFoundException {
-  {
     Class<?> clazz = getFromCache(name);
 
     if (clazz == null) {
@@ -87,8 +86,8 @@ public final class UberClassLoader extends ClassLoader
   }
 
   private Class<?> findClassInPlugins(String name) throws ClassNotFoundException {
-    for (InstalledPlugin plugin : plugins) {
+    for (ClassLoader pluginClassLoader : pluginClassLoaders) {
-      Class<?> clazz = findClass(plugin.getClassLoader(), name);
+      Class<?> clazz = findClass(pluginClassLoader, name);
       if (clazz != null) {
         return clazz;
       }
@@ -106,27 +105,14 @@ public final class UberClassLoader extends ClassLoader
     }
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param name
-   *
-   * @return
-   */
   @Override
-  protected URL findResource(String name)
+  protected URL findResource(String name) {
-  {
     URL url = null;
 
-    for (InstalledPlugin plugin : plugins)
+    for (ClassLoader pluginClassLoader : pluginClassLoaders) {
-    {
+      url = pluginClassLoader.getResource(name);
-      ClassLoader cl = plugin.getClassLoader();
 
-      url = cl.getResource(name);
+      if (url != null) {
-
-      if (url != null)
-      {
         break;
       }
     }
@@ -134,52 +120,26 @@ public final class UberClassLoader extends ClassLoader
     return url;
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param name
-   *
-   * @return
-   *
-   * @throws IOException
-   */
   @Override
-  protected Enumeration<URL> findResources(String name) throws IOException
+  @SuppressWarnings("squid:S2112")
-  {
+  protected Enumeration<URL> findResources(String name) throws IOException {
-    List<URL> urls = Lists.newArrayList();
+    Set<URL> urls = new LinkedHashSet<>();
 
-    for (InstalledPlugin plugin : plugins)
+    for (ClassLoader pluginClassLoader : pluginClassLoaders) {
-    {
+      urls.addAll(Collections.list(pluginClassLoader.getResources(name)));
-      ClassLoader cl = plugin.getClassLoader();
-
-      urls.addAll(Collections.list(cl.getResources(name)));
     }
 
     return Collections.enumeration(urls);
   }
 
-  //~--- get methods ----------------------------------------------------------
+  private Class<?> getFromCache(String name) {
-
-  /**
-   * Method description
-   *
-   *
-   * @param name
-   *
-   * @return
-   */
-  private Class<?> getFromCache(String name)
-  {
     Class<?> clazz = null;
     WeakReference<Class<?>> ref = cache.get(name);
 
-    if (ref != null)
+    if (ref != null) {
-    {
       clazz = ref.get();
 
-      if (clazz == null)
+      if (clazz == null) {
-      {
         cache.remove(name);
       }
     }
@@ -187,12 +147,4 @@ public final class UberClassLoader extends ClassLoader
     return clazz;
   }
 
-  //~--- fields ---------------------------------------------------------------
-
-  /** Field description */
-  private final ConcurrentMap<String, WeakReference<Class<?>>> cache =
-    Maps.newConcurrentMap();
-
-  /** Field description */
-  private final Iterable<InstalledPlugin> plugins;
 }

scm-webapp/src/test/java/sonia/scm/plugin/UberClassLoaderTest.java

@@ -0,0 +1,74 @@
+package sonia.scm.plugin;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.io.Resources;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junitpioneer.jupiter.TempDirectory;
+
+import java.io.IOException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.UUID;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@ExtendWith(TempDirectory.class)
+class UberClassLoaderTest {
+
+  private final URLClassLoader parentClassLoader = new URLClassLoader(new URL[0]);
+
+  @Test
+  void shouldOnlyUseClassloaderOnce(@TempDirectory.TempDir Path tempDir) throws IOException {
+    ClassLoader mailClassLoader = createClassLoader(tempDir, "plugin.txt", "mail");
+    ClassLoader reviewClassLoader = createClassLoader(mailClassLoader, tempDir, "plugin.txt", "review");
+
+    UberClassLoader uberClassLoader = new UberClassLoader(parentClassLoader, ImmutableSet.of(mailClassLoader, reviewClassLoader));
+    List<URL> resources = Collections.list(uberClassLoader.findResources("plugin.txt"));
+
+    assertThat(resources).hasSize(2);
+    assertThat(toContent(resources)).containsOnly("mail", "review");
+  }
+
+  @Test
+  void shouldReturnResourceFromEachPluginClassLoader(@TempDirectory.TempDir Path tempDir) throws IOException {
+    ClassLoader mailClassLoader = createClassLoader(tempDir, "scm.txt", "mail");
+    ClassLoader reviewClassLoader = createClassLoader(tempDir, "scm.txt", "review");
+
+    UberClassLoader uberClassLoader = new UberClassLoader(parentClassLoader, ImmutableSet.of(mailClassLoader, reviewClassLoader));
+    List<URL> resources = Collections.list(uberClassLoader.findResources("scm.txt"));
+    assertThat(toContent(resources)).containsOnly("mail", "review");
+  }
+
+  @SuppressWarnings("UnstableApiUsage")
+  private List<String> toContent(Iterable<URL> resources) throws IOException {
+    List<String> content = new ArrayList<>();
+    for (URL resource : resources) {
+      content.add(Resources.toString(resource, StandardCharsets.UTF_8));
+    }
+    return content;
+  }
+
+  private ClassLoader createClassLoader(Path tempDir, String resource, String value) throws IOException {
+    return createClassLoader(Thread.currentThread().getContextClassLoader(), tempDir, resource, value);
+  }
+
+  private ClassLoader createClassLoader(ClassLoader parent, Path tempDir, String resource, String value) throws IOException {
+    Path directory = tempDir.resolve(UUID.randomUUID().toString());
+    Files.createDirectory(directory);
+
+    Files.write(directory.resolve(resource), value.getBytes(StandardCharsets.UTF_8));
+
+    return new URLClassLoader(new URL[]{
+      directory.toUri().toURL()
+    }, parent);
+  }
+
+
+}