GEODE-4722: Refactor CliUtil.

* Replace .stackTraceToString and .contains with Apache Commons methods. * Moved getClientIdFromCacheClientProxy to only class that calls it. * Move arrayToString from CliUtil to sensible StringUtils. Moved associated test. * Refactor CliUtil methods.
apache · Feb 22, 2018 · f1eb507 · f1eb507
1 parent e056612
commit f1eb507
Show file tree

Hide file tree

Showing 27 changed files with 128 additions and 171 deletions.
diff --git a/...nectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/ExceptionHandler.java b/...nectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/ExceptionHandler.java
@@ -16,6 +16,7 @@
 
 import java.io.Serializable;
 
+import org.apache.commons.lang.exception.ExceptionUtils;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.cache.Cache;
@@ -51,7 +52,7 @@ private String getMember(final Cache cache) {
   private String getExceptionMessage(final Exception exception) {
     String message = exception.getMessage();
     if (message == null) {
-      message = CliUtil.stackTraceAsString(exception);
+      message = ExceptionUtils.getStackTrace(exception);
     }
     return message;
   }

diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
@@ -34,6 +34,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.CancelException;
@@ -75,7 +76,6 @@
 import org.apache.geode.internal.net.SocketCreatorFactory;
 import org.apache.geode.management.internal.JmxManagerLocator;
 import org.apache.geode.management.internal.JmxManagerLocatorRequest;
-import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.configuration.domain.SharedConfigurationStatus;
 import org.apache.geode.management.internal.configuration.handlers.SharedConfigurationStatusRequestHandler;
 import org.apache.geode.management.internal.configuration.messages.SharedConfigurationStatusRequest;
@@ -1123,7 +1123,8 @@ public SharedConfigurationStatusResponse getSharedConfigurationStatus() {
     try {
       response = statusFuture.get(5, TimeUnit.SECONDS);
     } catch (Exception e) {
-      logger.info("Exception occurred while fetching the status {}", CliUtil.stackTraceAsString(e));
+      logger.info("Exception occurred while fetching the status {}",
+          ExceptionUtils.getStackTrace(e));
       response = new SharedConfigurationStatusResponse();
       response.setStatus(SharedConfigurationStatus.UNDETERMINED);
     }

diff --git a/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java b/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java
@@ -18,6 +18,8 @@
 import java.util.Map;
 import java.util.Map.Entry;
 
+import org.apache.commons.lang.exception.ExceptionUtils;
+
 import org.apache.geode.Statistics;
 import org.apache.geode.StatisticsType;
 import org.apache.geode.cache.CacheWriterException;
@@ -34,7 +36,6 @@
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.internal.offheap.annotations.Released;
-import org.apache.geode.management.internal.cli.CliUtil;
 
 /**
  * This class publishes the client statistics using the admin region.
@@ -255,7 +256,7 @@ private static ClientHealthStats getClientHealthStats(InternalCache currentCache
 
     } catch (Exception e) {
       logger.fine("Exception in getting pool stats in  getClientHealthStats "
-          + CliUtil.stackTraceAsString(e));
+          + ExceptionUtils.getStackTrace(e));
     }
 
     stats.setUpdateTime(new Date());

diff --git a/geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java b/geode-core/src/main/java/org/apache/geode/internal/lang/StringUtils.java
@@ -15,6 +15,9 @@
 
 package org.apache.geode.internal.lang;
 
+import java.util.Arrays;
+import java.util.stream.Collectors;
+
 import org.apache.geode.internal.cache.CachedDeserializable;
 import org.apache.geode.internal.cache.Token;
 
@@ -192,6 +195,16 @@ public static String objectToString(Object o, boolean convertArrayContents,
     }
   }
 
+  /**
+   * Unlike the other arrayToString methods in this class, this method does not surround
+   * the array with "[" "]" delimiters.
+   */
+  public static <T> String arrayToString(T[] array) {
+    if (array == null) {
+      return "null";
+    }
+    return Arrays.stream(array).map(String::valueOf).collect(Collectors.joining(", "));
+  }
 
   private static <T> String arrayToString(T[] a, int maxArrayElements) {
     if (maxArrayElements < 0) {

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
@@ -62,7 +62,6 @@
 import org.apache.geode.management.internal.beans.stats.StatsAverageLatency;
 import org.apache.geode.management.internal.beans.stats.StatsKey;
 import org.apache.geode.management.internal.beans.stats.StatsRate;
-import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.membership.ClientMembershipListener;
 
 /**
@@ -713,7 +712,7 @@ private ClientQueueDetail getClientQueueDetail(CacheClientProxy p) {
     if (!p.isConnected() && !proxyID.isDurable()) {
       return null;
     }
-    queueDetail.setClientId(CliUtil.getClientIdFromCacheClientProxy(p));
+    queueDetail.setClientId(getClientIdFromCacheClientProxy(p));
 
     HARegionQueue queue = p.getHARegionQueue();
     if (queue == null) {
@@ -737,7 +736,7 @@ public ClientQueueDetail getClientQueueDetail(String clientId) throws Exception
         Collection<CacheClientProxy> clientProxies =
             acceptor.getCacheClientNotifier().getClientProxies();
         for (CacheClientProxy p : clientProxies) {
-          String buffer = CliUtil.getClientIdFromCacheClientProxy(p);
+          String buffer = getClientIdFromCacheClientProxy(p);
           if (buffer.equals(clientId)) {
             ClientQueueDetail queueDetail = getClientQueueDetail(p);
             return queueDetail;
@@ -750,4 +749,15 @@ public ClientQueueDetail getClientQueueDetail(String clientId) throws Exception
     return null;
   }
 
+
+  private static String getClientIdFromCacheClientProxy(CacheClientProxy p) {
+    if (p == null) {
+      return null;
+    }
+    StringBuffer buffer = new StringBuffer();
+    buffer.append("[").append(p.getProxyID()).append(":port=").append(p.getRemotePort())
+        .append(":primary=").append(p.isPrimary()).append("]");
+    return buffer.toString();
+  }
+
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
@@ -23,17 +23,16 @@
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.PrintWriter;
 import java.io.Serializable;
-import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.zip.DataFormatException;
@@ -55,7 +54,6 @@
 import org.apache.geode.internal.ClassPathLoader;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.execute.AbstractExecution;
-import org.apache.geode.internal.cache.tier.sockets.CacheClientProxy;
 import org.apache.geode.internal.util.IOUtils;
 import org.apache.geode.management.DistributedRegionMXBean;
 import org.apache.geode.management.ManagementService;
@@ -89,43 +87,28 @@ public static InternalCache getCacheIfExists() {
   }
 
   public static String cliDependenciesExist(boolean includeGfshDependencies) {
-    String jarProductName;
-
-    // Parser & CliCommand from Spring Shell
-    jarProductName =
-        checkLibraryByLoadingClass("org.springframework.shell.core.Parser", "Spring Shell");
-    jarProductName = checkLibraryByLoadingClass(
-        "org.springframework.shell.core.annotation.CliCommand", "Spring Shell");
-    if (jarProductName != null) {
-      return jarProductName;
-    }
-
-    // SpringVersion from Spring Core
-    jarProductName =
-        checkLibraryByLoadingClass("org.springframework.core.SpringVersion", "Spring Core");
-    if (jarProductName != null) {
-      return jarProductName;
-    }
-
+    // "Validate" each dependency by attempting to load an associated class
+    Map<String, String> classLibraryMap = new HashMap<>();
+    classLibraryMap.put("org.springframework.shell.core.Parser", "Spring Shell");
+    classLibraryMap.put("org.springframework.shell.core.annotation.CliCommand", "Spring Shell");
+    classLibraryMap.put("org.springframework.core.SpringVersion", "Spring Core");
     if (includeGfshDependencies) {
-      // ConsoleReader from jline
-      jarProductName = checkLibraryByLoadingClass("jline.console.ConsoleReader", "JLine");
-      if (jarProductName != null) {
-        return jarProductName;
-      }
+      classLibraryMap.put("jline.console.ConsoleReader", "JLine");
     }
 
-    return jarProductName;
+    List<String> unloadableJars =
+        classLibraryMap.entrySet().stream().filter(entry -> !canLoadClass(entry.getKey()))
+            .map(Map.Entry::getValue).collect(Collectors.toList());
+    return unloadableJars.isEmpty() ? null : String.join(",", unloadableJars);
   }
 
-  private static String checkLibraryByLoadingClass(String className, String jarProductName) {
+  private static boolean canLoadClass(String className) {
     try {
       ClassPathLoader.getLatest().forName(className);
     } catch (ClassNotFoundException e) {
-      return jarProductName;
+      return false;
     }
-
-    return null;
+    return true;
   }
 
   /**
@@ -157,8 +140,7 @@ public static Set<DistributedMember> getRegionAssociatedMembers(String region,
 
     String[] regionAssociatedMemberNames = regionMXBean.getMembers();
     Set<DistributedMember> matchedMembers = new HashSet<>();
-    Set<DistributedMember> allClusterMembers = new HashSet<>();
-    allClusterMembers.addAll(cache.getMembers());
+    Set<DistributedMember> allClusterMembers = new HashSet<>(cache.getMembers());
     allClusterMembers.add(cache.getDistributedSystem().getDistributedMember());
 
     for (DistributedMember member : allClusterMembers) {
@@ -369,15 +351,12 @@ public static Byte[][] filesToBytes(List<String> fileNames) throws IOException {
       }
     }
 
-    List<Byte[]> convertedList =
-        filesDataList.stream().map(ArrayUtils::toObject).collect(Collectors.toList());
-
-    return convertedList.toArray(new Byte[convertedList.size()][]);
+    return filesDataList.stream().map(ArrayUtils::toObject).toArray(Byte[][]::new);
   }
 
   public static byte[] toByteArray(InputStream input) throws IOException {
     ByteArrayOutputStream output = new ByteArrayOutputStream();
-    int n = 0;
+    int n;
     byte[] buffer = new byte[4096];
     while (-1 != (n = input.read(buffer))) {
       output.write(buffer, 0, n);
@@ -420,7 +399,7 @@ public static DeflaterInflaterData compressBytes(byte[] input) {
     compresser.finish();
     byte[] buffer = new byte[100];
     byte[] result = new byte[0];
-    int compressedDataLength = 0;
+    int compressedDataLength;
     int totalCompressedDataLength = 0;
     do {
       byte[] newResult = new byte[result.length + buffer.length];
@@ -461,7 +440,6 @@ public static String decodeWithDefaultCharSet(String urlToDecode) {
     }
   }
 
-  @SuppressWarnings("unchecked")
   public static <K> Class<K> forName(String classToLoadName, String neededFor) {
     Class<K> loadedClass = null;
     try {
@@ -558,7 +536,8 @@ public static Result getFunctionResult(ResultCollector<?, ?> rc, String commandN
     return execution.execute(function);
   }
 
-  public static void runLessCommandAsExternalViewer(Result commandResult, boolean isError) {
+
+  public static void runLessCommandAsExternalViewer(Result commandResult) {
     StringBuilder sb = new StringBuilder();
     String NEW_LINE = System.getProperty("line.separator");
 
@@ -629,75 +608,11 @@ public byte[] getData() {
     public String toString() {
       return String.valueOf(dataLength);
     }
-
   }
 
   // Methods that will be removed by the next commit:
 
   private static InternalCache getInternalCache() {
     return (InternalCache) CacheFactory.getAnyInstance();
   }
-
-  public static String convertStringSetToString(Set<String> stringSet, char delimiter) {
-    StringBuilder sb = new StringBuilder();
-    if (stringSet != null) {
-
-      for (String stringValue : stringSet) {
-        sb.append(stringValue);
-        sb.append(delimiter);
-      }
-    }
-    return sb.toString();
-  }
-
-  public static String convertStringListToString(List<String> stringList, char delimiter) {
-    StringBuilder sb = new StringBuilder();
-    if (stringList != null) {
-
-      for (String stringValue : stringList) {
-        sb.append(stringValue);
-        sb.append(delimiter);
-      }
-    }
-    return sb.toString();
-  }
-
-  public static String stackTraceAsString(Throwable e) {
-    String stackAsString = "";
-    if (e != null) {
-      StringWriter writer = new StringWriter();
-      PrintWriter pw = new PrintWriter(writer);
-      e.printStackTrace(pw);
-      stackAsString = writer.toString();
-    }
-    return stackAsString;
-  }
-
-  public static boolean contains(Object[] array, Object object) {
-    boolean contains = false;
-
-    if (array != null && object != null) {
-      contains = Arrays.asList(array).contains(object);
-    }
-
-    return contains;
-  }
-
-  public static <T> String arrayToString(T[] array) {
-    if (array == null) {
-      return "null";
-    }
-    return Arrays.stream(array).map(String::valueOf).collect(Collectors.joining(", "));
-  }
-
-  public static String getClientIdFromCacheClientProxy(CacheClientProxy p) {
-    if (p == null) {
-      return null;
-    }
-    StringBuffer buffer = new StringBuffer();
-    buffer.append("[").append(p.getProxyID()).append(":port=").append(p.getRemotePort())
-        .append(":primary=").append(p.isPrimary()).append("]");
-    return buffer.toString();
-  }
-
 }
diff --git a/...e-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java b/...e-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java
@@ -14,6 +14,8 @@
  */
 package org.apache.geode.management.internal.cli;
 
+import org.apache.commons.lang.exception.ExceptionUtils;
+
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.internal.cli.json.GfJsonException;
 import org.apache.geode.management.internal.cli.json.GfJsonObject;
@@ -40,7 +42,7 @@ public static CommandResponse prepareCommandResponseFromJson(String jsonString)
     try {
       jsonObject = new GfJsonObject(jsonString);
     } catch (GfJsonException e) {
-      jsonObject = GfJsonObject.getGfJsonErrorObject(CliUtil.stackTraceAsString(e));
+      jsonObject = GfJsonObject.getGfJsonErrorObject(ExceptionUtils.getStackTrace(e));
     }
     return new CommandResponse(jsonObject);
   }

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/Launcher.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/Launcher.java
@@ -27,6 +27,7 @@
 import org.apache.geode.internal.ExitCode;
 import org.apache.geode.internal.GemFireVersion;
 import org.apache.geode.internal.PureJavaMode;
+import org.apache.geode.internal.lang.StringUtils;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.shell.Gfsh;
 import org.apache.geode.management.internal.cli.shell.GfshConfig;
@@ -168,7 +169,7 @@ private int parseCommandLineCommand(final String... args) {
 
         if (!commandIsAllowed) {
           System.err.println(
-              CliStrings.format(MSG_INVALID_COMMAND_OR_OPTION, CliUtil.arrayToString(args)));
+              CliStrings.format(MSG_INVALID_COMMAND_OR_OPTION, StringUtils.arrayToString(args)));
           exitRequest = ExitShellRequest.FATAL_EXIT;
         } else {
           if (!gfsh.executeScriptLine(commandLineCommand)) {
@@ -189,8 +190,8 @@ private int parseOptions(final String... args) {
     try {
       parsedOptions = this.commandLineParser.parse(args);
     } catch (OptionException e) {
-      System.err
-          .println(CliStrings.format(MSG_INVALID_COMMAND_OR_OPTION, CliUtil.arrayToString(args)));
+      System.err.println(
+          CliStrings.format(MSG_INVALID_COMMAND_OR_OPTION, StringUtils.arrayToString(args)));
       return ExitShellRequest.FATAL_EXIT.getExitCode();
     }
     boolean launchShell = true;