Skip to content

QL: Extend NodeSubclass to read classes from jars #50866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 15, 2020
Merged

QL: Extend NodeSubclass to read classes from jars #50866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e8ffa1
Extend NodeSubclass to read classes from jars
costin Jan 10, 2020
a9386b2
Fix forbiddenApi
costin Jan 10, 2020
9ef7c2b
Keep the results in a Set (instead of List)
costin Jan 14, 2020
e0c1462
Merge remote-tracking branch 'remotes/upstream/master' into ql/fix-te…
costin Jan 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 72 additions & 34 deletions x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/tree/NodeSubclassTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.ql.expression.Expression;
Expand Down Expand Up @@ -52,10 +53,14 @@
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;

import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;
Expand Down Expand Up @@ -603,60 +608,93 @@ public <T extends Node<?>> T makeNode(Class<? extends T> nodeClass) throws Excep
* Cache of subclasses. We use a cache because it significantly speeds up
* the test.
*/
private static final Map<Class<?>, List<?>> subclassCache = new HashMap<>();
private static final Map<Class<?>, Set<?>> subclassCache = new HashMap<>();

/**
* Find all subclasses of a particular class.
*/
public static <T> List<Class<? extends T>> subclassesOf(Class<T> clazz) throws IOException {
public static <T> Set<Class<? extends T>> subclassesOf(Class<T> clazz) throws IOException {
@SuppressWarnings("unchecked") // The map is built this way
List<Class<? extends T>> lookup = (List<Class<? extends T>>) subclassCache.get(clazz);
Set<Class<? extends T>> lookup = (Set<Class<? extends T>>) subclassCache.get(clazz);
if (lookup != null) {
return lookup;
}
List<Class<? extends T>> results = new ArrayList<>();
Set<Class<? extends T>> results = new LinkedHashSet<>();
String[] paths = System.getProperty("java.class.path").split(System.getProperty("path.separator"));
for (String path: paths) {
Path root = PathUtils.get(path);
int rootLength = root.toString().length() + 1;
Files.walkFileTree(root, new SimpleFileVisitor<Path>() {

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
if (Files.isRegularFile(file) && file.getFileName().toString().endsWith(".class")) {
String className = file.toString();
// Chop off the root and file extension
className = className.substring(rootLength, className.length() - ".class".length());
// Go from "path" style to class style
className = className.replace(PathUtils.getDefaultFileSystem().getSeparator(), ".");

// filter the class that are not interested
// (and IDE folders like eclipse)
if (!className.startsWith("org.elasticsearch.xpack.ql") && !className.startsWith("org.elasticsearch.xpack.sql")) {
return FileVisitResult.CONTINUE;
}

Class<?> c;
try {
c = Class.forName(className);
} catch (ClassNotFoundException e) {
throw new IOException("Couldn't find " + file, e);
}

if (false == Modifier.isAbstract(c.getModifiers())
&& false == c.isAnonymousClass()
&& clazz.isAssignableFrom(c)) {
Class<? extends T> s = c.asSubclass(clazz);
results.add(s);
// load classes from jar files
// NIO FileSystem API is not used since it trips the SecurityManager
// https://bugs.openjdk.java.net/browse/JDK-8160798
// so iterate the jar "by hand"
if (path.endsWith(".jar") && path.contains("x-pack-ql")) {
try (JarInputStream jar = jarStream(root)) {
JarEntry je = null;
while ((je = jar.getNextJarEntry()) != null) {
String name = je.getName();
if (name.endsWith(".class")) {
String className = name.substring(0, name.length() - ".class".length()).replace("/", ".");
maybeLoadClass(clazz, className, root + "!/" + name, results);
Copy link
Contributor

@bpintea bpintea Jan 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is !/ intended as a visual marker in the path? The location is later only used for the message in the Exception, so I guess so, but curious about it.

Copy link
Member Author

@costin costin Jan 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's a special marker inside the URL to indicate the path inside the archive:
jar:/my/file.jar!/org/my/package/clazz.class

}
}
return FileVisitResult.CONTINUE;
}
});
}
// for folders, just use the FileSystems API
else {
Files.walkFileTree(root, new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
if (Files.isRegularFile(file) && file.getFileName().toString().endsWith(".class")) {
String fileName = file.toString();
// Chop off the root and file extension
String className = fileName.substring(rootLength, fileName.length() - ".class".length());
// Go from "path" style to class style
className = className.replace(PathUtils.getDefaultFileSystem().getSeparator(), ".");
maybeLoadClass(clazz, className, fileName, results);
}
return FileVisitResult.CONTINUE;
}
});
}
}
subclassCache.put(clazz, results);
return results;
}

@SuppressForbidden(reason = "test reads from jar")
private static JarInputStream jarStream(Path path) throws IOException {
return new JarInputStream(path.toUri().toURL().openStream());
}

/**
* Load classes from predefined packages (hack to limit the scope) and if they match the hierarchy, add them to the cache
*/
private static <T> void maybeLoadClass(Class<T> clazz, String className, String location, Set<Class<? extends T>> results)
throws IOException {

// filter the class that are not interested
// (and IDE folders like eclipse)
if (className.startsWith("org.elasticsearch.xpack.ql") == false && className.startsWith("org.elasticsearch.xpack.sql") == false) {
return;
}

Class<?> c;
try {
c = Class.forName(className);
} catch (ClassNotFoundException e) {
throw new IOException("Couldn't load " + location, e);
}

if (false == Modifier.isAbstract(c.getModifiers())
&& false == c.isAnonymousClass()
&& clazz.isAssignableFrom(c)) {
Class<? extends T> s = c.asSubclass(clazz);
results.add(s);
}
}

/**
* The test class for some subclass of node or {@code null}
* if there isn't such a class or it doesn't extend
Expand Down
4 changes: 1 addition & 3 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/ProcessorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

package org.elasticsearch.xpack.sql.expression;

import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
import org.elasticsearch.common.io.stream.NamedWriteable;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.ql.expression.gen.processor.Processor;
Expand All @@ -22,10 +21,9 @@

import static java.util.stream.Collectors.toCollection;

@AwaitsFix(bugUrl = "classpath inside tests is different")
public class ProcessorTests extends ESTestCase {

private static List<Class<? extends Processor>> processors;
private static Set<Class<? extends Processor>> processors;

@BeforeClass
public static void init() throws Exception {
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/tree/SqlNodeSubclassTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/
package org.elasticsearch.xpack.sql.tree;

import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.Literal;
import org.elasticsearch.xpack.ql.expression.LiteralTests;
Expand Down Expand Up @@ -48,7 +47,6 @@
* node of that type is called for.
* </ul>
*/
@AwaitsFix(bugUrl = "classpath inside tests is different")
public class SqlNodeSubclassTests<T extends B, B extends Node<B>> extends NodeSubclassTests<T, B> {

private static final List<Class<?>> CLASSES_WITH_MIN_TWO_CHILDREN = CollectionUtils.combine(
Expand Down