From 04d480684ad85c6f91d767da83467513e88e3603 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 3 Mar 2020 10:39:07 +0100 Subject: [PATCH] prevent using same classloader multiple times --- .../sonia/scm/plugin/UberClassLoader.java | 132 ++++++------------ .../sonia/scm/plugin/UberClassLoaderTest.java | 74 ++++++++++ 2 files changed, 116 insertions(+), 90 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/plugin/UberClassLoaderTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/UberClassLoader.java b/scm-webapp/src/main/java/sonia/scm/plugin/UberClassLoader.java index 62b1073a85..411fa8173c 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/UberClassLoader.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/UberClassLoader.java @@ -1,9 +1,9 @@ /** * Copyright (c) 2010, Sebastian Sdorra All rights reserved. - * + *

* Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: - * + *

* 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. - * + *

* 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. - * + *

* 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 { - /** - * Constructs ... - * - * - * @param parent - * @param plugins - */ - public UberClassLoader(ClassLoader parent, Iterable plugins) - { - super(parent); - this.plugins = plugins; + private final Set pluginClassLoaders; + private final ConcurrentMap>> cache = Maps.newConcurrentMap(); + + public UberClassLoader(ClassLoader parent, Iterable plugins) { + this(parent, collectClassLoaders(plugins)); } - //~--- methods -------------------------------------------------------------- + private static Set collectClassLoaders(Iterable plugins) { + ImmutableSet.Builder classLoaders = ImmutableSet.builder(); + plugins.forEach(plugin -> classLoaders.add(plugin.getClassLoader())); + return classLoaders.build(); + } + + @VisibleForTesting + UberClassLoader(ClassLoader parent, Set 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) { - Class clazz = findClass(plugin.getClassLoader(), name); + for (ClassLoader pluginClassLoader : pluginClassLoaders) { + 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) - { - ClassLoader cl = plugin.getClassLoader(); + for (ClassLoader pluginClassLoader : pluginClassLoaders) { + url = pluginClassLoader.getResource(name); - 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 findResources(String name) throws IOException - { - List urls = Lists.newArrayList(); + @SuppressWarnings("squid:S2112") + protected Enumeration findResources(String name) throws IOException { + Set urls = new LinkedHashSet<>(); - for (InstalledPlugin plugin : plugins) - { - ClassLoader cl = plugin.getClassLoader(); - - urls.addAll(Collections.list(cl.getResources(name))); + for (ClassLoader pluginClassLoader : pluginClassLoaders) { + urls.addAll(Collections.list(pluginClassLoader.getResources(name))); } return Collections.enumeration(urls); } - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @param name - * - * @return - */ - private Class getFromCache(String name) - { + private Class getFromCache(String name) { Class clazz = null; WeakReference> 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>> cache = - Maps.newConcurrentMap(); - - /** Field description */ - private final Iterable plugins; } diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/UberClassLoaderTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/UberClassLoaderTest.java new file mode 100644 index 0000000000..319e534c56 --- /dev/null +++ b/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 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 resources = Collections.list(uberClassLoader.findResources("scm.txt")); + assertThat(toContent(resources)).containsOnly("mail", "review"); + } + + @SuppressWarnings("UnstableApiUsage") + private List toContent(Iterable resources) throws IOException { + List 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); + } + + +}