From 8864cb284877aa8e2dafb2ca25d6d8b3b82c9ced Mon Sep 17 00:00:00 2001 From: Mike Percy Date: Wed, 2 Jan 2019 16:35:19 -0800 Subject: [PATCH] java: KuduBinaryExtractor should use the thread context classloader Using the thread context classloader makes it straightforward to write end-to-end tests, one of which is included in this patch. This patch makes the following changes: 1. The KuduBinaryExtractor will now search for the test binary jars via the thread context classloader, if available. 2. Remove the work-in-progress OS-detection implementation from KuduBinaryExtractor (to be replaced in a future commit) because it was failing tests. 3. KuduBinaryExtractor will no longer cache its search results to make it more straightforward to use a thread local context classloader. 4. KuduBinaryExtractor.extractKuduBinary() now throws FileNotFoundException instead of IllegalStateException when the Kudu binary test jar cannot be found. Since it is a checked exception and a subclass of IOException it will be less likely to go uncaught. 5. Update the API docs to be more specific about the semantics of the public methods of KuduBinaryExtractor, including the use of the thread context classloader. 6. Add a simple test binary locator test using a child classloader plumbed into the KuduBinaryExtractor code by setting it as the thread context classloader. Change-Id: I5e1cf188bb557eeaea0b2867243855f3f2d121f1 Reviewed-on: http://gerrit.cloudera.org:8080/12147 Tested-by: Mike Percy Reviewed-by: Brian McDevitt Reviewed-by: Grant Henke --- .../test/cluster/KuduBinaryJarExtractor.java | 51 ++++++++++++------- .../cluster/TestKuduBinaryJarExtractor.java | 29 ++++++++++- 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryJarExtractor.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryJarExtractor.java index 3a599f9c5b..006bfbd999 100644 --- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryJarExtractor.java +++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryJarExtractor.java @@ -49,24 +49,25 @@ public class KuduBinaryJarExtractor { private static final String KUDU_TEST_BIN_PROPS_PATH = "META-INF/apache-kudu-test-binary.properties"; - protected Properties binaryProps; + public KuduBinaryJarExtractor() {} - public KuduBinaryJarExtractor() throws IOException { - this.binaryProps = getBinaryProps(); + /** Return the thread context classloader or the parent classloader for this class. */ + private static ClassLoader getCurrentClassLoader() { + ClassLoader loader = Thread.currentThread().getContextClassLoader(); + if (loader != null) return loader; + return KuduBinaryJarExtractor.class.getClassLoader(); } private static Properties getBinaryProps() throws IOException { - Enumeration resources = - KuduBinaryJarExtractor.class.getClassLoader().getResources(KUDU_TEST_BIN_PROPS_PATH); + Enumeration resources = getCurrentClassLoader().getResources(KUDU_TEST_BIN_PROPS_PATH); //TODO: normalize osName //TODO: check for matching architecture - String osName = System.getProperties().getProperty("os.name").toLowerCase().replace(" ", ""); while (resources.hasMoreElements()) { URL url = resources.nextElement(); try { - Properties props = loadBinaryProps(url); - if (osName.startsWith(props.getProperty("artifact.os"))) { - return props; + Properties binaryProps = loadBinaryProps(url); + if (binaryProps != null && !binaryProps.isEmpty()) { + return binaryProps; } } catch (IOException ex) { LOG.warn("Unable to parse properties file from Kudu binary artifact", ex); @@ -82,31 +83,43 @@ private static Properties loadBinaryProps(URL url) throws IOException { } /** - * Determine if the classpath has a Kudu binary jar that matches the system's OS and CPU - * architecture + * Determine if the classpath has a Kudu binary test jar compatible with the system architecture + * and operating system. + * If a Thread context ClassLoader is set, then that ClassLoader is searched. + * Otherwise, the ClassLoader that loaded this class is searched. * - * @return true if an appropriate Kudu binary jar is available, false otherwise + *

TODO: at the time of writing, OS and architecture checks are not yet implemented. + * + * @return {@code true} if an appropriate Kudu binary jar is available, {@code false} otherwise */ - public boolean isKuduBinaryJarOnClasspath() { - return null != binaryProps; + public boolean isKuduBinaryJarOnClasspath() throws IOException { + Properties binaryProps = getBinaryProps(); + return binaryProps != null; } /** - * Extract the Kudu binary jar found on the classpath. + * Extract the Kudu binary test jar found on the classpath to the specified location. + * If a Thread context ClassLoader is set, then that ClassLoader is searched. + * Otherwise, the ClassLoader that loaded this class is searched. + * + *

It is expected that + * {@link #isKuduBinaryJarOnClasspath()} should return {@code true} before this method is invoked. * * @param destDir path to a destination - * @return the absolute path to the directory containing extracted executables. + * @return the absolute path to the directory containing extracted executables, * eg. "/tmp/apache-kudu-1.9.0/bin" - * @throws IOException if any of the JAR extraction process throws an exception + * @throws FileNotFoundException if the binary JAR cannot not be located + * @throws IOException if the JAR extraction process fails */ public String extractKuduBinary(String destDir) throws IOException { + Properties binaryProps = getBinaryProps(); if (binaryProps == null) { - throw new IllegalStateException("Could not locate the Kudu binary jar"); + throw new FileNotFoundException("Could not locate the Kudu binary test jar"); } try { String prefix = binaryProps.getProperty("artifact.prefix"); - URL kuduBinDir = KuduBinaryJarExtractor.class.getClassLoader().getResource(prefix); + URL kuduBinDir = getCurrentClassLoader().getResource(prefix); if (null == kuduBinDir) { throw new FileNotFoundException("Cannot find Kudu binary dir: " + prefix); } diff --git a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java index b29cf5a901..91f623bedd 100644 --- a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java +++ b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java @@ -24,6 +24,8 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.net.URL; +import java.net.URLClassLoader; import java.nio.file.FileSystem; import java.nio.file.FileSystems; import java.nio.file.FileVisitResult; @@ -36,6 +38,7 @@ import java.util.HashMap; import java.util.Map; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -48,7 +51,7 @@ private Path createKuduBinaryJar() throws IOException, URISyntaxException { // convert the filename to a URI final Path path = Paths.get(tempDir.toString(), "fake-kudu-binary.jar"); - LOG.info("Creating kudu-fake-binar.jar at {}", path.toString()); + LOG.info("Creating fake kudu binary jar at {}", path.toString()); final URI uri = URI.create("jar:file:" + path.toUri().getPath()); final Map env = new HashMap<>(); @@ -85,6 +88,20 @@ public FileVisitResult preVisitDirectory(Path dir, return path; } + /** + * Create a ClassLoader. The parent of the ClassLoader will be the current thread context + * ClassLoader, if not set, or the ClassLoader that loaded this test class if not. + * @param jars an array of jars to include in the child class loader. + */ + private ClassLoader createChildClassLoader(URL[] jars) { + ClassLoader parent = Thread.currentThread().getContextClassLoader(); + if (parent == null) { + parent = TestKuduBinaryJarExtractor.class.getClassLoader(); + } + assertNotNull(parent); + return URLClassLoader.newInstance(jars, parent); + } + @Test public void testExtractJar() throws IOException, URISyntaxException { Path binaryJar = createKuduBinaryJar(); @@ -98,4 +115,14 @@ public void testExtractJar() throws IOException, URISyntaxException { Path kuduTserver = Paths.get(extractedBinDir.toString(), "kudu-tserver"); assertTrue(Files.exists(kuduTserver)); } + + @Test + public void testIsKuduBinaryJarOnClasspath() throws IOException, URISyntaxException { + KuduBinaryJarExtractor extractor = new KuduBinaryJarExtractor(); + assertFalse(extractor.isKuduBinaryJarOnClasspath()); + Path binaryJar = createKuduBinaryJar(); + ClassLoader childLoader = createChildClassLoader(new URL[] { binaryJar.toUri().toURL() }); + Thread.currentThread().setContextClassLoader(childLoader); + assertTrue(extractor.isKuduBinaryJarOnClasspath()); + } }