You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2019/01/03 17:40:38 UTC
[2/2] kudu git commit: java: KuduBinaryExtractor should use the
thread context classloader
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 <mp...@apache.org>
Reviewed-by: Brian McDevitt <br...@phdata.io>
Reviewed-by: Grant Henke <gr...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/8864cb28
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/8864cb28
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/8864cb28
Branch: refs/heads/master
Commit: 8864cb284877aa8e2dafb2ca25d6d8b3b82c9ced
Parents: 2fa5884
Author: Mike Percy <mp...@apache.org>
Authored: Wed Jan 2 16:35:19 2019 -0800
Committer: Mike Percy <mp...@apache.org>
Committed: Thu Jan 3 17:40:16 2019 +0000
----------------------------------------------------------------------
.../test/cluster/KuduBinaryJarExtractor.java | 51 ++++++++++++--------
.../cluster/TestKuduBinaryJarExtractor.java | 29 ++++++++++-
2 files changed, 60 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/8864cb28/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryJarExtractor.java
----------------------------------------------------------------------
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 3a599f9..006bfbd 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<URL> resources =
- KuduBinaryJarExtractor.class.getClassLoader().getResources(KUDU_TEST_BIN_PROPS_PATH);
+ Enumeration<URL> 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 @@ public class KuduBinaryJarExtractor {
}
/**
- * 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
+ * <p>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.
+ *
+ * <p>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);
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8864cb28/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java
----------------------------------------------------------------------
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 b29cf5a..91f623b 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 org.slf4j.LoggerFactory;
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.nio.file.attribute.BasicFileAttributes;
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 @@ public class TestKuduBinaryJarExtractor {
// 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<String, String> env = new HashMap<>();
@@ -85,6 +88,20 @@ public class TestKuduBinaryJarExtractor {
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 class TestKuduBinaryJarExtractor {
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());
+ }
}