geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jstew...@apache.org
Subject [1/5] geode git commit: GEODE-2989: Improve mechanism for scanning the classpath to find gfsh commands
Date Thu, 01 Jun 2017 16:06:19 GMT
Repository: geode
Updated Branches:
  refs/heads/develop 72b935183 -> 9046c8dcd


GEODE-2989: Improve mechanism for scanning the classpath to find gfsh commands


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/2f50aef9
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/2f50aef9
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/2f50aef9

Branch: refs/heads/develop
Commit: 2f50aef97311df301cc6492a20954babb68f3ee3
Parents: 72b9351
Author: Jared Stewart <jstewart@pivotal.io>
Authored: Fri May 26 11:28:10 2017 -0700
Committer: Jared Stewart <jstewart@pivotal.io>
Committed: Thu Jun 1 08:51:17 2017 -0700

----------------------------------------------------------------------
 .../management/internal/cli/CommandManager.java |  81 +++----
 .../cli/util/ClasspathScanLoadHelper.java       | 226 ++-----------------
 .../cli/ClasspathScanLoadHelperJUnitTest.java   |  30 +--
 .../cli/shell/GfshInitFileJUnitTest.java        |  36 +--
 .../security/MemberMBeanSecurityJUnitTest.java  |   8 +-
 .../security/PDXPostProcessorDUnitTest.java     |  14 +-
 .../test/dunit/rules/ServerStarterRule.java     |  11 +
 .../ShellCommandsControllerJUnitTest.java       |  21 +-
 8 files changed, 109 insertions(+), 318 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
----------------------------------------------------------------------
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 0576e46..f28f9f6 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
@@ -16,9 +16,12 @@ package org.apache.geode.management.internal.cli;
 
 import static org.apache.geode.distributed.ConfigurationProperties.USER_COMMAND_PACKAGES;
 
+import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
+import org.apache.commons.lang.exception.ExceptionUtils;
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.ClassPathLoader;
+import org.apache.geode.management.internal.cli.commands.AbstractCommandsSupport;
 import org.apache.geode.management.internal.cli.help.Helper;
 import org.apache.geode.management.internal.cli.shell.Gfsh;
 import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
@@ -29,7 +32,6 @@ import org.springframework.shell.core.MethodTarget;
 import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
 import org.springframework.shell.core.annotation.CliCommand;
 
-import java.io.IOException;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -125,8 +127,8 @@ public class CommandManager {
     // Load commands found in all of the packages
     for (String userCommandPackage : userCommandPackages) {
       try {
-        Set<Class<?>> foundClasses =
-            ClasspathScanLoadHelper.loadAndGet(userCommandPackage, CommandMarker.class, true);
+        Set<Class<?>> foundClasses = ClasspathScanLoadHelper
+            .scanPackageForClassesImplementing(userCommandPackage, CommandMarker.class);
         for (Class<?> klass : foundClasses) {
           try {
             add((CommandMarker) klass.newInstance());
@@ -136,8 +138,6 @@ public class CommandManager {
           }
         }
         raiseExceptionIfEmpty(foundClasses, "User Command");
-      } catch (ClassNotFoundException | IOException e) {
-        logWrapper.warning("Could not load User Commands due to " + e.getLocalizedMessage());
       } catch (IllegalStateException e) {
         logWrapper.warning(e.getMessage(), e);
         throw e;
@@ -168,42 +168,21 @@ public class CommandManager {
     }
   }
 
+
   private void loadCommands() {
     loadUserCommands();
 
     loadPluginCommands();
+    loadGeodeCommands();
+    loadConverters();
+  }
 
-    // CommandMarkers
-    Set<Class<?>> foundClasses = null;
-    try {
-      // geode's commands
-      foundClasses = ClasspathScanLoadHelper.loadAndGet(
-          "org.apache.geode.management.internal.cli.commands", CommandMarker.class, true);
-      for (Class<?> klass : foundClasses) {
-        try {
-          add((CommandMarker) klass.newInstance());
-        } catch (Exception e) {
-          logWrapper.warning(
-              "Could not load Command from: " + klass + " due to " + e.getLocalizedMessage());
// continue
-        }
-      }
-      raiseExceptionIfEmpty(foundClasses, "Commands");
-
-      // do not add Spring shell's commands for now. When we add it, we need to tell the
parser that
-      // these are offline commands.
-    } catch (ClassNotFoundException e) {
-      logWrapper.warning("Could not load Commands due to " + e.getLocalizedMessage());
-    } catch (IOException e) {
-      logWrapper.warning("Could not load Commands due to " + e.getLocalizedMessage());
-    } catch (IllegalStateException e) {
-      logWrapper.warning(e.getMessage(), e);
-      throw e;
-    }
-
+  private void loadConverters() {
+    Set<Class<?>> foundClasses;
     // Converters
     try {
-      foundClasses = ClasspathScanLoadHelper
-          .loadAndGet("org.apache.geode.management.internal.cli.converters", Converter.class,
true);
+      foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
+          "org.apache.geode.management.internal.cli.converters", Converter.class);
       for (Class<?> klass : foundClasses) {
         try {
           Converter<?> object = (Converter<?>) klass.newInstance();
@@ -217,8 +196,8 @@ public class CommandManager {
       raiseExceptionIfEmpty(foundClasses, "Converters");
 
       // Spring shell's converters
-      foundClasses = ClasspathScanLoadHelper.loadAndGet("org.springframework.shell.converters",
-          Converter.class, true);
+      foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
+          "org.springframework.shell.converters", Converter.class);
       for (Class<?> klass : foundClasses) {
         if (!SHL_CONVERTERS_TOSKIP.contains(klass)) {
           try {
@@ -230,10 +209,32 @@ public class CommandManager {
         }
       }
       raiseExceptionIfEmpty(foundClasses, "Basic Converters");
-    } catch (ClassNotFoundException e) {
-      logWrapper.warning("Could not load Converters due to " + e.getLocalizedMessage());
-    } catch (IOException e) {
-      logWrapper.warning("Could not load Converters due to " + e.getLocalizedMessage());
+    } catch (IllegalStateException e) {
+      logWrapper.warning(e.getMessage(), e);
+      throw e;
+    }
+  }
+
+  private void loadGeodeCommands() {
+    // CommandMarkers
+    Set<Class<?>> foundClasses;
+    try {
+      // geode's commands
+      foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
+          AbstractCommandsSupport.class.getPackage().getName(), CommandMarker.class);
+
+      for (Class<?> klass : foundClasses) {
+        try {
+          add((CommandMarker) klass.newInstance());
+        } catch (Exception e) {
+          logWrapper.warning(
+              "Could not load Command from: " + klass + " due to " + e.getLocalizedMessage());
// continue
+        }
+      }
+      raiseExceptionIfEmpty(foundClasses, "Commands");
+
+      // do not add Spring shell's commands for now. When we add it, we need to tell the
parser that
+      // these are offline commands.
     } catch (IllegalStateException e) {
       logWrapper.warning(e.getMessage(), e);
       throw e;

http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
----------------------------------------------------------------------
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 20fffbd..d3bc7d3 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
@@ -14,234 +14,34 @@
  */
 package org.apache.geode.management.internal.cli.util;
 
-import java.io.File;
-import java.io.FileFilter;
-import java.io.FileInputStream;
-import java.io.IOException;
+import static java.util.stream.Collectors.toSet;
+
+import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
+
 import java.lang.reflect.Modifier;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Enumeration;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
-import java.util.jar.JarEntry;
-import java.util.jar.JarInputStream;
-
-import org.apache.geode.internal.ClassPathLoader;
-import org.apache.geode.management.internal.cli.CliUtil;
 
 /**
  * Utility class to scan class-path & load classes.
  * 
- * 
  * @since GemFire 7.0
  */
 public class ClasspathScanLoadHelper {
-  private static final String CLASSFILE_EXTENSION = ".class";
-
-  public static Set<Class<?>> loadAndGet(String commandPackageName,
-      Class<?> requiredInterfaceToLoad, boolean onlyInstantiable)
-      throws ClassNotFoundException, IOException {
+  public static Set<Class<?>> scanPackageForClassesImplementing(String packageToScan,
+      Class<?> implementedInterface) {
+    Set<Class<?>> classesImplementing = new HashSet<>();
+    new FastClasspathScanner(packageToScan)
+        .matchClassesImplementing(implementedInterface, classesImplementing::add).scan();
 
-    Set<Class<?>> classSet = new HashSet<Class<?>>();
-    Class<?> classes[] = getClasses(commandPackageName);
-
-    for (int i = 0; i < classes.length; i++) {
-      if (implementsType(classes[i], requiredInterfaceToLoad)) {
-        if (onlyInstantiable) {
-          if (isInstantiable(classes[i])) {
-            classSet.add(classes[i]);
-          }
-        } else {
-          classSet.add(classes[i]);
-        }
-      }
-    }
-
-    return classSet;
+    return classesImplementing.stream().filter(ClasspathScanLoadHelper::isInstantiable)
+        .collect(toSet());
   }
 
-  public static boolean isInstantiable(Class<?> klass) {
+  private static boolean isInstantiable(Class<?> klass) {
     int modifiers = klass.getModifiers();
 
-    boolean isInstantiable = !Modifier.isAbstract(modifiers) && !Modifier.isInterface(modifiers)
+    return !Modifier.isAbstract(modifiers) && !Modifier.isInterface(modifiers)
         && Modifier.isPublic(modifiers);
-
-    return isInstantiable;
-  }
-
-  private static boolean implementsType(Class<?> typeToCheck, Class<?> requiredInterface)
{
-    if (requiredInterface.isAssignableFrom(typeToCheck)) {
-      return true;
-    } else {
-      return false;
-    }
-  }
-
-  public static Class<?>[] getClasses(String packageName)
-      throws ClassNotFoundException, IOException {
-    String packagePath = packageName.replace('.', '/');
-
-    List<File> dirs = new ArrayList<File>();
-
-    Enumeration<URL> resources = ClassPathLoader.getLatest().getResources(packagePath);
-    List<Class<?>> classesList = new ArrayList<Class<?>>();
-
-    while (resources.hasMoreElements()) {
-      URL packageUrl = resources.nextElement();
-
-      String actualPackagePath = packageUrl.getPath();
-      int jarIndex = actualPackagePath.indexOf(".jar!");
-      if (jarIndex != -1) { // resource appears to be in a jar
-        String jarPath = actualPackagePath.substring(0, jarIndex + ".jar".length());
-
-        if (jarPath.startsWith("file:")) {
-          if (File.separatorChar == '/') {// whether Unix or Windows system
-            // On Unix, to get actual path, we remove "file:" from the Path
-            jarPath = jarPath.substring("file:".length());
-          } else {
-            // On Windows jarPaths are like:
-            // Local Path:
-            // file:/G:/where/java/spring/spring-shell/1.0.0/spring-shell-1.0.0.RELEASE.jar
-            // Network Path:
-            // file://stinger.pune.gemstone.com/shared/where/java/spring/spring-shell/1.0.0/spring-shell-1.0.0.RELEASE.jar
-            // To get actual path, we remove "file:/" from the Path
-            jarPath = jarPath.substring("file:/".length());
-            // If the path still starts with a "/", then it's a network path.
-            // Hence, add one "/".
-            if (jarPath.startsWith("/") && !jarPath.startsWith("//")) {
-              jarPath = "/" + jarPath;
-            }
-          }
-        }
-        // decode the jarPath as it's derived from an URL
-        Class<?>[] classes = getClasses(CliUtil.decodeWithDefaultCharSet(jarPath),
packageName);
-        classesList.addAll(Arrays.asList(classes));
-      } else {
-        dirs.add(new File(packageUrl.getFile()));
-      }
-    }
-
-    for (File directory : dirs) {
-      classesList.addAll(findClasses(directory, packageName));
-    }
-
-    return (Class[]) classesList.toArray(new Class[classesList.size()]);
-  }
-
-  public static List<Class<?>> findClasses(File directory, String packageName)
-      throws ClassNotFoundException {
-    List<Class<?>> classes = new ArrayList<Class<?>>();
-    if (!directory.exists()) {
-      return classes;
-    }
-
-    ClassPathLoader cpLoader = ClassPathLoader.getLatest();
-    // Load only .class files that are not from test code
-    TestClassFilter tcf = new TestClassFilter();
-
-    File[] files = directory.listFiles(tcf);
-    File file = null;
-    for (int i = 0; i < files.length; i++) {
-      file = files[i];
-      if (file.isDirectory()) {// sub-package
-        // assert !file.getName().contains(".");
-        classes.addAll(findClasses(file, packageName + "." + file.getName()));
-      } else {
-        // remove .class from the file name
-        String classSimpleName =
-            file.getName().substring(0, file.getName().length() - CLASSFILE_EXTENSION.length());
-        classes.add(cpLoader.forName(packageName + '.' + classSimpleName));
-      }
-    }
-    return classes;
-  }
-
-
-  /**
-   * Returns all classes that are in the specified jar and package name.
-   * 
-   * @param jarPath The absolute or relative jar path.
-   * @param packageName The package name.
-   * @return Returns all classes that are in the specified jar and package name.
-   * @throws ClassNotFoundException Thrown if unable to load a class
-   * @throws IOException Thrown if error occurs while reading the jar file
-   */
-  public static Class<?>[] getClasses(String jarPath, String packageName)
-      throws ClassNotFoundException, IOException {
-    ClassPathLoader cpLoader = ClassPathLoader.getLatest();
-
-    String[] classNames = getClassNames(jarPath, packageName);
-    Class<?> classes[] = new Class[classNames.length];
-    for (int i = 0; i < classNames.length; i++) {
-      String className = (String) classNames[i];
-      classes[i] = cpLoader.forName(className);
-    }
-    return classes;
-  }
-
-  /**
-   * Returns all names of classes that are defined in the specified jar and package name.
-   * 
-   * @param jarPath The absolute or relative jar path.
-   * @param packageName The package name.
-   * @return Returns all names of classes that are defined in the specified jar and package
name.
-   * @throws IOException Thrown if error occurs while reading the jar file
-   */
-  public static String[] getClassNames(String jarPath, String packageName) throws IOException
{
-    if (jarPath == null) {
-      return new String[0];
-    }
-
-    File file;
-    // Path is absolute on Unix if it starts with '/'
-    // or path contains colon on Windows
-    if (jarPath.startsWith("/") || (jarPath.indexOf(':') >= 0 && File.separatorChar
== '\\')) {
-      // absolute path
-      file = new File(jarPath);
-    } else {
-      // relative path
-      String workingDir = System.getProperty("user.dir");
-      file = new File(workingDir + File.separator + jarPath);
-    }
-
-    List<String> classNames = new ArrayList<String>();
-    String packagePath = packageName.replaceAll("\\.", "/");
-    JarInputStream jarFile = new JarInputStream(new FileInputStream(file));
-    JarEntry jarEntry;
-    while (true) {
-      jarEntry = jarFile.getNextJarEntry();
-      if (jarEntry == null) {
-        break;
-      }
-      String name = jarEntry.getName();
-      if (name.startsWith(packagePath) && (name.endsWith(CLASSFILE_EXTENSION))) {
-        int endIndex = name.length() - 6;
-        name = name.replaceAll("/", "\\.");
-        name = name.substring(0, endIndex);
-        classNames.add(name);
-      }
-    }
-    jarFile.close();
-
-    return (String[]) classNames.toArray(new String[0]);
-  }
-
-  /**
-   * FileFilter to filter out GemFire Test Code.
-   * 
-   * @since GemFire 7.0
-   */
-  static class TestClassFilter implements FileFilter {
-    private static final String TESTS_CODE_INDICATOR = "Test";
-
-    @Override
-    public boolean accept(File pathname) {
-      String pathToCheck = pathname.getName();
-      return !pathToCheck.contains(TESTS_CODE_INDICATOR)
-          && pathToCheck.endsWith(CLASSFILE_EXTENSION);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
----------------------------------------------------------------------
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 a13ca35..827eb9c 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
@@ -18,6 +18,7 @@ import static org.junit.Assert.*;
 
 import java.util.Set;
 
+import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -27,7 +28,6 @@ import org.apache.geode.management.internal.cli.domain.Impl1;
 import org.apache.geode.management.internal.cli.domain.Impl12;
 import org.apache.geode.management.internal.cli.domain.Interface1;
 import org.apache.geode.management.internal.cli.domain.Interface2;
-import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
 import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
@@ -44,35 +44,27 @@ public class ClasspathScanLoadHelperJUnitTest {
 
   @Test
   public void testLoadAndGet() throws Exception {
-    Set<Class<?>> classLoaded = ClasspathScanLoadHelper.loadAndGet(PACKAGE_NAME,
INTERFACE1, true);
+    Set<Class<?>> classLoaded =
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, INTERFACE1);
     assertEquals(2, classLoaded.size());
     assertTrue(classLoaded.contains(IMPL1));
     assertTrue(classLoaded.contains(IMPL2));
-    // impl1 and impl12
-
-    classLoaded = ClasspathScanLoadHelper.loadAndGet(PACKAGE_NAME, INTERFACE1, false);
-    assertEquals(4, classLoaded.size());
-    assertTrue(classLoaded.contains(IMPL1));
-    assertTrue(classLoaded.contains(IMPL2));
-    assertTrue(classLoaded.contains(ABSTRACT_IMPL));
-    assertTrue(classLoaded.contains(INTERFACE1));
-
-    classLoaded = ClasspathScanLoadHelper.loadAndGet(PACKAGE_NAME, INTERFACE2, false);
-    assertEquals(2, classLoaded.size());
-    assertTrue(classLoaded.contains(IMPL2));
-    assertTrue(classLoaded.contains(INTERFACE2));
 
-    classLoaded = ClasspathScanLoadHelper.loadAndGet(PACKAGE_NAME, INTERFACE2, true);
+    classLoaded =
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, INTERFACE2);
     assertEquals(1, classLoaded.size());
     assertTrue(classLoaded.contains(IMPL2));
 
-    classLoaded = ClasspathScanLoadHelper.loadAndGet(WRONG_PACKAGE_NAME, INTERFACE2, true);
+    classLoaded =
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME, INTERFACE2);
     assertEquals(0, classLoaded.size());
 
-    classLoaded = ClasspathScanLoadHelper.loadAndGet(PACKAGE_NAME, NO_IMPL_INTERFACE, true);
+    classLoaded =
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, NO_IMPL_INTERFACE);
     assertEquals(0, classLoaded.size());
 
-    classLoaded = ClasspathScanLoadHelper.loadAndGet(WRONG_PACKAGE_NAME, NO_IMPL_INTERFACE,
true);
+    classLoaded = ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME,
+        NO_IMPL_INTERFACE);
     assertEquals(0, classLoaded.size());
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
index 159c47f..5d61333 100755
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
@@ -36,6 +36,7 @@ import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 import org.junit.experimental.categories.Category;
 import org.junit.rules.TemporaryFolder;
 
@@ -57,8 +58,6 @@ public class GfshInitFileJUnitTest {
   private static final int INIT_FILE_CITATION_LINES = 1;
 
   private static String saveLog4j2Config;
-  private static String saveUserDir;
-  private static String saveUserHome;
   private static java.util.logging.Logger julLogger;
   private static Handler[] saveHandlers;
 
@@ -73,6 +72,9 @@ public class GfshInitFileJUnitTest {
   @Rule
   public TemporaryFolder temporaryFolder_CurrentDirectory = new TemporaryFolder();
 
+  @Rule
+  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+
   /*
    * Turn off console logging from JUL and Log4j2 for the duration of this class's tests,
to reduce
    * noise level of output in automated build.
@@ -80,8 +82,6 @@ public class GfshInitFileJUnitTest {
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
     saveLog4j2Config = System.getProperty(ConfigurationFactory.CONFIGURATION_FILE_PROPERTY);
-    saveUserDir = System.getProperty("user.dir");
-    saveUserHome = System.getProperty("user.home");
 
     julLogger = java.util.logging.Logger.getLogger("");
     saveHandlers = julLogger.getHandlers();
@@ -110,38 +110,10 @@ public class GfshInitFileJUnitTest {
       System.setProperty(ConfigurationFactory.CONFIGURATION_FILE_PROPERTY, saveLog4j2Config);
       ((LoggerContext) LogManager.getContext(false)).reconfigure();
     }
-
-    if (saveUserDir == null) {
-      System.clearProperty("user.dir");
-    } else {
-      System.setProperty("user.dir", saveUserDir);
-    }
-    if (saveUserHome == null) {
-      System.clearProperty("user.home");
-    } else {
-      System.setProperty("user.home", saveUserHome);
-    }
   }
 
-  /*
-   * Reset $HOME and current directory for tests, and confirm this has been accepted.
-   */
   @Before
   public void setUp() throws Exception {
-    // Fake running from home directory
-    String userDir = temporaryFolder_CurrentDirectory.getRoot().getAbsolutePath();
-    String userHome = userDir;
-
-    System.setProperty("user.dir", userDir);
-    System.setProperty("user.home", userHome);
-
-    // Abort all tests if system properties cannot be overridden
-    assertEquals("user.dir", System.getProperty("user.dir"), userDir);
-    assertEquals("user.home", System.getProperty("user.home"), userHome);
-  }
-
-  @Before
-  public void setUp2() throws Exception {
 
     // Null out static instance so can reinitialise
     Field gfsh_instance = Gfsh.class.getDeclaredField("instance");

http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
index 65fd528..6ae8d3f 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
@@ -27,15 +27,19 @@ import org.apache.geode.test.junit.categories.SecurityTest;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 import org.junit.experimental.categories.Category;
 
 @Category({IntegrationTest.class, SecurityTest.class})
 public class MemberMBeanSecurityJUnitTest {
   private MemberMXBean bean;
 
+  @Rule
+  public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+
   @Rule // do not use a ClassRule since some test will do a shutdownMember
-  public ServerStarterRule server = new ServerStarterRule().withJMXManager()
-      .withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName())
+  public ServerStarterRule server = ServerStarterRule.createWithoutTemporaryWorkingDir()
+      .withJMXManager().withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName())
       .withProperty(TestSecurityManager.SECURITY_JSON,
           "org/apache/geode/management/internal/security/cacheServer.json")
       .withAutoStart();

http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
index e952386..134e33f 100644
--- a/geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
@@ -80,13 +80,13 @@ public class PDXPostProcessorDUnitTest extends JUnit4DistributedTestCase
{
   }
 
   @Rule
-  public ServerStarterRule server =
-      new ServerStarterRule().withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName())
-          .withProperty(TestSecurityManager.SECURITY_JSON,
-              "org/apache/geode/management/internal/security/clientServer.json")
-          .withProperty(SECURITY_POST_PROCESSOR, PDXPostProcessor.class.getName())
-          .withProperty("security-pdx", pdxPersistent + "").withJMXManager()
-          .withRegion(RegionShortcut.REPLICATE, REGION_NAME);
+  public ServerStarterRule server = ServerStarterRule.createWithoutTemporaryWorkingDir()
+      .withProperty(SECURITY_MANAGER, TestSecurityManager.class.getName())
+      .withProperty(TestSecurityManager.SECURITY_JSON,
+          "org/apache/geode/management/internal/security/clientServer.json")
+      .withProperty(SECURITY_POST_PROCESSOR, PDXPostProcessor.class.getName())
+      .withProperty("security-pdx", pdxPersistent + "").withJMXManager()
+      .withRegion(RegionShortcut.REPLICATE, REGION_NAME);
 
   @Test
   public void testRegionGet() {

http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
index 30ae59f..40cfe99 100644
--- a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
+++ b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
@@ -93,6 +93,17 @@ public class ServerStarterRule extends MemberStarterRule<ServerStarterRule>
impl
     }
   }
 
+  /**
+   * By default, the ServerStartRule dynamically changes the "user.dir" system property to
point to
+   * a temporary folder. The Path API caches the first value of "user.dir" that it sees,
and this
+   * can result in a stale cached value of "user.dir" which points to a directory that no
longer
+   * exists, causing later tests to fail. By passing in the real value of "user.dir", we
avoid these
+   * problems.
+   */
+  public static ServerStarterRule createWithoutTemporaryWorkingDir() {
+    return new ServerStarterRule(new File(System.getProperty("user.dir")));
+  }
+
   @Override
   public void stopMember() {
     // make sure this cache is the one currently open. A server cache can be recreated due
to

http://git-wip-us.apache.org/repos/asf/geode/blob/2f50aef9/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java
b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java
index 10e26f6..2f9214c 100644
--- a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java
+++ b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerJUnitTest.java
@@ -14,11 +14,13 @@
  */
 package org.apache.geode.management.internal.web.controllers;
 
+import static java.util.stream.Collectors.toSet;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
 import org.apache.commons.lang.StringUtils;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
@@ -42,6 +44,7 @@ import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -78,8 +81,8 @@ public class ShellCommandsControllerJUnitTest {
 
   private List<String> getCliCommands() {
     try {
-      Set<Class<?>> commandClasses = ClasspathScanLoadHelper.loadAndGet(
-          "org.apache.geode.management.internal.cli.commands", CommandMarker.class, true);
+      Set<Class<?>> commandClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
+          "org.apache.geode.management.internal.cli.commands", CommandMarker.class);
 
       List<String> commands = new ArrayList<>(commandClasses.size());
 
@@ -107,9 +110,8 @@ public class ShellCommandsControllerJUnitTest {
     String scheme = servletRequest.getScheme();
 
     try {
-      Set<Class<?>> controllerClasses =
-          ClasspathScanLoadHelper.loadAndGet("org.apache.geode.management.internal.web.controllers",
-              AbstractCommandsController.class, true);
+      Set<Class<?>> controllerClasses = scanPackageForClassesExtending(
+          "org.apache.geode.management.internal.web.controllers", AbstractCommandsController.class);
 
       List<String> controllerWebServiceEndpoints = new ArrayList<>(controllerClasses.size());
 
@@ -239,4 +241,13 @@ public class ShellCommandsControllerJUnitTest {
     assertTrue(String.format("Link does not have correct scheme %1$s", linkIndex.find(versionCmd)),
         testScheme.equals(linkIndex.find(versionCmd).getHref().getScheme()));
   }
+
+  public static Set<Class<?>> scanPackageForClassesExtending(String packageToScan,
+      Class<?> superclass) {
+    Set<Class<?>> classesImplementing = new HashSet<>();
+    new FastClasspathScanner(packageToScan).matchSubclassesOf(superclass, classesImplementing::add)
+        .scan();
+
+    return classesImplementing.stream().collect(toSet());
+  }
 }


Mime
View raw message