You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2018/08/12 23:41:03 UTC

[geode] branch develop updated: GEODE-5535: Upgrade FastClasspathScanner to 4.0.6 (#2310)

This is an automated email from the ASF dual-hosted git repository.

jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 6d60c0a  GEODE-5535: Upgrade FastClasspathScanner to 4.0.6 (#2310)
6d60c0a is described below

commit 6d60c0a27e020d1ff8419fdceb1dbd0e288179d2
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Sun Aug 12 16:40:57 2018 -0700

    GEODE-5535: Upgrade FastClasspathScanner to 4.0.6 (#2310)
    
    - Use less memory for scanning by reusing a single
      instance of ClassGraph.
---
 LICENSE                                            |  2 +
 geode-assembly/build.gradle                        |  2 +-
 .../integrationTest/resources/expected_jars.txt    |  2 +-
 geode-core/build.gradle                            |  3 +-
 .../InternalConfigurationPersistenceService.java   | 15 +++--
 .../management/internal/cli/CommandManager.java    | 65 +++++++++++++---------
 .../internal/cli/util/ClasspathScanLoadHelper.java | 50 +++++++++--------
 .../internal/deployment/FunctionScanner.java       | 18 +++---
 ...nternalConfigurationPersistenceServiceTest.java | 10 ++--
 .../cli/ClasspathScanLoadHelperJUnitTest.java      | 19 +++++--
 gradle/dependency-versions.properties              |  2 +-
 11 files changed, 107 insertions(+), 81 deletions(-)

diff --git a/LICENSE b/LICENSE
index 0dc431d..a243c40 100644
--- a/LICENSE
+++ b/LICENSE
@@ -257,6 +257,8 @@ The MIT License (http://opensource.org/licenses/mit-license.html)
 
 Apache Geode bundles the following files under the MIT license:
 
+  - ClassGraph v4.0.6 (https://github.com/classgraph/classgraph), Copyright (c)
+    2015 Luke Hutchison
   - HTML5 Shiv vpre3.5 (https://github.com/aFarkas/html5shiv), Copyright
     (c) 2014 Alexander Farkas (aFarkas)
   - JavaScript InfoVis Toolkit v2.0.1 (http://philogb.github.io/jit/),
diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
index cfea7e2..84b2aa7 100755
--- a/geode-assembly/build.gradle
+++ b/geode-assembly/build.gradle
@@ -181,13 +181,13 @@ def cp = {
         // depedencies from geode-core
         it.contains('antlr') ||
         it.contains('commons-io') ||
+        it.contains('classgraph') ||
         it.contains('commons-collections') ||
         it.contains('commons-lang') ||
         it.contains('commons-logging') ||
         it.contains('commons-validator') ||
         it.contains('commons-beanutils') ||
         it.contains('commons-codec') ||
-        it.contains('fast-classpath-scanner') ||
         it.contains('fastutil') ||
         it.contains('jackson-annotations') ||
         it.contains('jackson-core') ||
diff --git a/geode-assembly/src/integrationTest/resources/expected_jars.txt b/geode-assembly/src/integrationTest/resources/expected_jars.txt
index 6418396..480c12c 100644
--- a/geode-assembly/src/integrationTest/resources/expected_jars.txt
+++ b/geode-assembly/src/integrationTest/resources/expected_jars.txt
@@ -2,6 +2,7 @@ activation
 antlr
 aopalliance
 byte-buddy
+classgraph
 classmate
 commons-beanutils
 commons-codec
@@ -13,7 +14,6 @@ commons-lang
 commons-logging
 commons-modeler
 commons-validator
-fast-classpath-scanner
 fastutil
 findbugs-annotations
 gfsh-dependencies.jar
diff --git a/geode-core/build.gradle b/geode-core/build.gradle
index f432b52..5ce12fb 100755
--- a/geode-core/build.gradle
+++ b/geode-core/build.gradle
@@ -132,8 +132,7 @@ dependencies {
 
   compile 'org.apache.shiro:shiro-core:' + project.'shiro.version'
 
-  // https://mvnrepository.com/artifact/io.github.lukehutch/fast-classpath-scanner
-  compile 'io.github.lukehutch:fast-classpath-scanner:' + project.'fast-classpath-scanner.version'
+  compile 'io.github.classgraph:classgraph:' + project.'classgraph.version'
 
   compile 'com.healthmarketscience.rmiio:rmiio:' + project.'rmiio.version'
 
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
index fae9b74..14a8f83 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
@@ -180,19 +180,22 @@ public class InternalConfigurationPersistenceService implements ConfigurationPer
     }
     // else, scan the classpath to find all the classes annotated with XSDRootElement
     else {
-      String[] packages = getPackagesToScan();
-      Set<Class<?>> scannedClasses =
-          ClasspathScanLoadHelper.scanClasspathForAnnotation(XSDRootElement.class, packages);
+      Set<String> packages = getPackagesToScan();
+      ClasspathScanLoadHelper scanner = new ClasspathScanLoadHelper(packages);
+      Set<Class<?>> scannedClasses = scanner.scanClasspathForAnnotation(XSDRootElement.class,
+          packages.toArray(new String[] {}));
       this.jaxbService = new JAXBService(scannedClasses.toArray(new Class[scannedClasses.size()]));
     }
     jaxbService.validateWithLocalCacheXSD();
   }
 
-  protected String[] getPackagesToScan() {
+  protected Set<String> getPackagesToScan() {
+    Set<String> packages = new HashSet<>();
     String sysProperty = SystemPropertyHelper.getProperty(SystemPropertyHelper.PACKAGES_TO_SCAN);
-    String[] packages = {""};
     if (sysProperty != null) {
-      packages = sysProperty.split(",");
+      packages = Arrays.stream(sysProperty.split(",")).collect(Collectors.toSet());
+    } else {
+      packages.add("org.apache.geode");
     }
     return packages;
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
index 354115c..0a29965 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
@@ -95,8 +95,8 @@ public class CommandManager {
     }
   }
 
-  private void loadUserCommands() {
-    final Set<String> userCommandPackages = new HashSet<String>();
+  private Set<String> getUserCommandPackages() {
+    final Set<String> userCommandPackages = new HashSet<>();
 
     List<String> userCommandSources = new ArrayList<>();
     // Find by packages specified by the system property
@@ -122,24 +122,30 @@ public class CommandManager {
       Arrays.stream(source.split(",")).forEach(userCommandPackages::add);
     }
 
+    return userCommandPackages;
+  }
+
+  private void loadUserCommands(ClasspathScanLoadHelper scanner, Set<String> restrictedToPackages) {
+    if (restrictedToPackages.size() == 0) {
+      return;
+    }
+
     // Load commands found in all of the packages
-    for (String userCommandPackage : userCommandPackages) {
-      try {
-        Set<Class<?>> foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(
-            CommandMarker.class, userCommandPackage, GfshCommand.class.getPackage().getName());
-        for (Class<?> klass : foundClasses) {
-          try {
-            add((CommandMarker) klass.newInstance());
-          } catch (Exception e) {
-            logWrapper.warning("Could not load User Commands from: " + klass + " due to "
-                + e.getLocalizedMessage()); // continue
-          }
+    try {
+      Set<Class<?>> foundClasses = scanner.scanPackagesForClassesImplementing(CommandMarker.class,
+          restrictedToPackages.toArray(new String[] {}));
+      for (Class<?> klass : foundClasses) {
+        try {
+          add((CommandMarker) klass.newInstance());
+        } catch (Exception e) {
+          logWrapper.warning("Could not load User Commands from: " + klass + " due to "
+              + e.getLocalizedMessage()); // continue
         }
-        raiseExceptionIfEmpty(foundClasses, "User Command");
-      } catch (IllegalStateException e) {
-        logWrapper.warning(e.getMessage(), e);
-        throw e;
       }
+      raiseExceptionIfEmpty(foundClasses, "User Command");
+    } catch (IllegalStateException e) {
+      logWrapper.warning(e.getMessage(), e);
+      throw e;
     }
   }
 
@@ -166,18 +172,27 @@ public class CommandManager {
   }
 
   private void loadCommands() {
-    loadUserCommands();
+    Set<String> userCommandPackages = getUserCommandPackages();
+    Set<String> packagesToScan = new HashSet<>(userCommandPackages);
+    packagesToScan.add("org.apache.geode.management.internal.cli.converters");
+    packagesToScan.add("org.springframework.shell.converters");
+    packagesToScan.add(GfshCommand.class.getPackage().getName());
+    packagesToScan.add(InternalGfshCommand.class.getPackage().getName());
+
+    // Create one scanner to be used everywhere
+    ClasspathScanLoadHelper scanner = new ClasspathScanLoadHelper(packagesToScan);
 
+    loadUserCommands(scanner, userCommandPackages);
     loadPluginCommands();
-    loadGeodeCommands();
-    loadConverters();
+    loadGeodeCommands(scanner);
+    loadConverters(scanner);
   }
 
-  private void loadConverters() {
+  private void loadConverters(ClasspathScanLoadHelper scanner) {
     Set<Class<?>> foundClasses;
     // Converters
     try {
-      foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(Converter.class,
+      foundClasses = scanner.scanPackagesForClassesImplementing(Converter.class,
           "org.apache.geode.management.internal.cli.converters");
       for (Class<?> klass : foundClasses) {
         try {
@@ -192,7 +207,7 @@ public class CommandManager {
       raiseExceptionIfEmpty(foundClasses, "Converters");
 
       // Spring shell's converters
-      foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(Converter.class,
+      foundClasses = scanner.scanPackagesForClassesImplementing(Converter.class,
           "org.springframework.shell.converters");
       for (Class<?> klass : foundClasses) {
         if (!SHL_CONVERTERS_TOSKIP.contains(klass)) {
@@ -211,12 +226,12 @@ public class CommandManager {
     }
   }
 
-  private void loadGeodeCommands() {
+  private void loadGeodeCommands(ClasspathScanLoadHelper scanner) {
     // CommandMarkers
     Set<Class<?>> foundClasses;
     try {
       // geode's commands
-      foundClasses = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(CommandMarker.class,
+      foundClasses = scanner.scanPackagesForClassesImplementing(CommandMarker.class,
           GfshCommand.class.getPackage().getName(),
           InternalGfshCommand.class.getPackage().getName());
 
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
index 08e1170..93c06c5 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
@@ -16,12 +16,13 @@ package org.apache.geode.management.internal.cli.util;
 
 import static java.util.stream.Collectors.toSet;
 
-import java.lang.reflect.Modifier;
-import java.util.HashSet;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Set;
 
-import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
-
+import io.github.classgraph.ClassGraph;
+import io.github.classgraph.ClassInfoList;
+import io.github.classgraph.ScanResult;
 
 /**
  * Utility class to scan class-path & load classes.
@@ -29,28 +30,33 @@ import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
  * @since GemFire 7.0
  */
 public class ClasspathScanLoadHelper {
-  public static Set<Class<?>> scanPackagesForClassesImplementing(Class<?> implementedInterface,
-      String... packagesToScan) {
-    Set<Class<?>> classesImplementing = new HashSet<>();
-    new FastClasspathScanner(packagesToScan)
-        .matchClassesImplementing(implementedInterface, classesImplementing::add).scan();
-
-    return classesImplementing.stream().filter(ClasspathScanLoadHelper::isInstantiable)
-        .collect(toSet());
+
+  private final ScanResult scanResult;
+
+  public ClasspathScanLoadHelper(Collection<String> packagesToScan) {
+    scanResult = new ClassGraph().whitelistPackages(packagesToScan.toArray(new String[] {}))
+        .enableClassInfo()
+        .enableAnnotationInfo().scan();
   }
 
-  public static Set<Class<?>> scanClasspathForAnnotation(Class<?> annotation,
-      String... packagesToScan) {
-    Set<Class<?>> classesWithAnnotation = new HashSet<>();
-    new FastClasspathScanner(packagesToScan)
-        .matchClassesWithAnnotation(annotation, classesWithAnnotation::add).scan();
-    return classesWithAnnotation;
+  public Set<Class<?>> scanPackagesForClassesImplementing(Class<?> implementedInterface,
+      String... onlyFromPackages) {
+    ClassInfoList classInfoList = scanResult.getClassesImplementing(implementedInterface.getName())
+        .filter(ci -> !ci.isAbstract() && !ci.isInterface() && ci.isPublic());
+
+    classInfoList = classInfoList
+        .filter(ci -> Arrays.stream(onlyFromPackages).anyMatch(p -> ci.getName().startsWith(p)));
+
+    return classInfoList.loadClasses().stream().collect(toSet());
   }
 
-  private static boolean isInstantiable(Class<?> klass) {
-    int modifiers = klass.getModifiers();
+  public Set<Class<?>> scanClasspathForAnnotation(Class<?> annotation, String... onlyFromPackages) {
+    ClassInfoList classInfoList = scanResult.getClassesWithAnnotation(annotation.getName());
+
+    classInfoList = classInfoList
+        .filter(ci -> Arrays.stream(onlyFromPackages).anyMatch(p -> ci.getName().startsWith(p)));
 
-    return !Modifier.isAbstract(modifiers) && !Modifier.isInterface(modifiers)
-        && Modifier.isPublic(modifiers);
+    return classInfoList.loadClasses().stream().collect(toSet());
   }
+
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/deployment/FunctionScanner.java b/geode-core/src/main/java/org/apache/geode/management/internal/deployment/FunctionScanner.java
index 3e4ec66..9108390 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/deployment/FunctionScanner.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/deployment/FunctionScanner.java
@@ -16,14 +16,12 @@ package org.apache.geode.management.internal.deployment;
 
 import java.io.File;
 import java.io.IOException;
-import java.net.URL;
-import java.net.URLClassLoader;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
 
-import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
-import io.github.lukehutch.fastclasspathscanner.scanner.ScanResult;
+import io.github.classgraph.ClassGraph;
+import io.github.classgraph.ScanResult;
 
 import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.FunctionAdapter;
@@ -31,16 +29,14 @@ import org.apache.geode.cache.execute.FunctionAdapter;
 public class FunctionScanner {
 
   public Collection<String> findFunctionsInJar(File jarFile) throws IOException {
-    URLClassLoader urlClassLoader =
-        new URLClassLoader(new URL[] {jarFile.getCanonicalFile().toURL()});
-    FastClasspathScanner fastClasspathScanner = new FastClasspathScanner("-dir:")
-        .removeTemporaryFilesAfterScan(true).overrideClassLoaders(urlClassLoader);
-    ScanResult scanResult = fastClasspathScanner.scan();
+    ClassGraph fastClasspathScanner = new ClassGraph().disableDirScanning()
+        .removeTemporaryFilesAfterScan().overrideClasspath(jarFile.getAbsolutePath());
+    ScanResult scanResult = fastClasspathScanner.enableClassInfo().scan();
 
     Set<String> functionClasses = new HashSet<>();
 
-    functionClasses.addAll(scanResult.getNamesOfClassesImplementing(Function.class));
-    functionClasses.addAll(scanResult.getNamesOfSubclassesOf(FunctionAdapter.class));
+    functionClasses.addAll(scanResult.getClassesImplementing(Function.class.getName()).getNames());
+    functionClasses.addAll(scanResult.getSubclasses(FunctionAdapter.class.getName()).getNames());
 
     return functionClasses;
   }
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java
index 6dbf018..281e682 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java
@@ -191,18 +191,16 @@ public class InternalConfigurationPersistenceServiceTest {
 
   @Test
   public void getPackagesToScanWithoutSystemProperty() {
-    String[] packages = service.getPackagesToScan();
-    assertThat(packages).hasSize(1);
-    assertThat(packages[0]).isEqualTo("");
+    Set<String> packages = service.getPackagesToScan();
+    assertThat(packages).containsExactly("org.apache.geode");
   }
 
   @Test
   public void getPackagesToScanWithSystemProperty() {
     System.setProperty("geode." + SystemPropertyHelper.PACKAGES_TO_SCAN,
         "org.apache.geode,io.pivotal");
-    String[] packages = service.getPackagesToScan();
-    assertThat(packages).hasSize(2);
-    assertThat(packages).contains("org.apache.geode", "io.pivotal");
+    Set<String> packages = service.getPackagesToScan();
+    assertThat(packages).containsExactly("org.apache.geode", "io.pivotal");
   }
 
   @Test
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
index 0972054..ba1b647 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
@@ -17,6 +17,7 @@ package org.apache.geode.management.internal.cli;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashSet;
 import java.util.Set;
 
 import org.junit.Test;
@@ -42,27 +43,33 @@ public class ClasspathScanLoadHelperJUnitTest {
 
   @Test
   public void testLoadAndGet() throws Exception {
+    Set<String> packages = new HashSet<String>() {
+      {
+        add(PACKAGE_NAME);
+        add(WRONG_PACKAGE_NAME);
+      }
+    };
+    ClasspathScanLoadHelper scanner = new ClasspathScanLoadHelper(packages);
     Set<Class<?>> classLoaded =
-        ClasspathScanLoadHelper.scanPackagesForClassesImplementing(INTERFACE1, PACKAGE_NAME);
+        scanner.scanPackagesForClassesImplementing(INTERFACE1, PACKAGE_NAME);
     assertEquals(2, classLoaded.size());
     assertTrue(classLoaded.contains(IMPL1));
     assertTrue(classLoaded.contains(IMPL2));
 
     classLoaded =
-        ClasspathScanLoadHelper.scanPackagesForClassesImplementing(INTERFACE2, PACKAGE_NAME);
+        scanner.scanPackagesForClassesImplementing(INTERFACE2, PACKAGE_NAME);
     assertEquals(1, classLoaded.size());
     assertTrue(classLoaded.contains(IMPL2));
 
     classLoaded =
-        ClasspathScanLoadHelper.scanPackagesForClassesImplementing(INTERFACE2, WRONG_PACKAGE_NAME);
+        scanner.scanPackagesForClassesImplementing(INTERFACE2, WRONG_PACKAGE_NAME);
     assertEquals(0, classLoaded.size());
 
     classLoaded =
-        ClasspathScanLoadHelper.scanPackagesForClassesImplementing(NO_IMPL_INTERFACE, PACKAGE_NAME);
+        scanner.scanPackagesForClassesImplementing(NO_IMPL_INTERFACE, PACKAGE_NAME);
     assertEquals(0, classLoaded.size());
 
-    classLoaded = ClasspathScanLoadHelper.scanPackagesForClassesImplementing(NO_IMPL_INTERFACE,
-        WRONG_PACKAGE_NAME);
+    classLoaded = scanner.scanPackagesForClassesImplementing(NO_IMPL_INTERFACE, WRONG_PACKAGE_NAME);
     assertEquals(0, classLoaded.size());
   }
 }
diff --git a/gradle/dependency-versions.properties b/gradle/dependency-versions.properties
index d9a1ae6..48ad4d5 100644
--- a/gradle/dependency-versions.properties
+++ b/gradle/dependency-versions.properties
@@ -21,6 +21,7 @@ bcel.version = 6.0
 catch-exception.version = 1.4.4
 catch-throwable.version = 1.4.4
 cglib.version = 3.2.4
+classgraph.version = 4.0.6
 commons-beanutils.version = 1.9.3
 commons-collections.version = 3.2.2
 commons-configuration.version = 1.10
@@ -35,7 +36,6 @@ commons-validator.version = 1.6
 HikariCP.version = 3.2.0
 derby.version = 10.13.1.1
 dom4j.version = 1.6.1
-fast-classpath-scanner.version=2.21
 fastutil.version = 8.2.1
 google-gson.version=2.8.5
 guava.version = 25.1-jre