GEODE-4517: Remove getAnyInstancce call from CliUtil.

* CliUtil.getCacheIfExists now takes supplier, but will squelch error and return null as before.
* LogWriter.getInstance now explicitly takes a Cache, to hook into the cache's LogWriter.
* GfshExecutionStrategy adopts the Gfsh shell's LogWriter, since the shell will necessarily have
* * finished instantiating the LogWriter's singleton before GfshExecutionStrategy can instantiate.
* In some instances, the LogWriter has been replaced with the Log4j LogService logger.
* Launcher explicitly passes a null Cache, since it does not yet exist.
* Other minor improvements: visibility, typo corrections, etc.

geode-core/src/main/java/org/apache/geode/internal/GfeConsoleReaderFactory.java

@@ -37,12 +37,13 @@ public static GfeConsoleReader getDefaultConsoleReader() {
   }
 
   public static GfeConsoleReader createConsoleReader() {
-    GfeConsoleReader consoleReader = null;
+    GfeConsoleReader consoleReader;
 
     if (Gfsh.getCurrentInstance() != null) {
-      LogWrapper.getInstance().info("GfeConsoleReaderFactory.createConsoleReader(): isGfshVM");
+      LogWrapper logWrapper = Gfsh.getCurrentInstance().getGfshFileLogger();
+      logWrapper.info("GfeConsoleReaderFactory.createConsoleReader(): isGfshVM");
       consoleReader = new GfshConsoleReader();
-      LogWrapper.getInstance().info("GfeConsoleReaderFactory.createConsoleReader(): consoleReader: "
+      logWrapper.info("GfeConsoleReaderFactory.createConsoleReader(): consoleReader: "
           + consoleReader + "=" + consoleReader.isSupported());
     } else {
       consoleReader = new GfeConsoleReader();

geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java

@@ -340,7 +340,7 @@ public MemberMBeanBridge(InternalCache cache, SystemManagementService service) {
     this.config = system.getConfig();
     try {
       this.commandProcessor =
-          new OnlineCommandProcessor(system.getProperties(), cache.getSecurityService());
+          new OnlineCommandProcessor(system.getProperties(), cache.getSecurityService(), cache);
     } catch (Exception e) {
       commandServiceInitError = e.getMessage();
       logger.info(LogMarker.CONFIG, "Command processor could not be initialized. {}",

geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java

@@ -34,16 +34,16 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.zip.DataFormatException;
 import java.util.zip.Deflater;
 import java.util.zip.Inflater;
 
 import org.apache.commons.lang.ArrayUtils;
-import org.apache.commons.lang.StringUtils;
 
+import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheClosedException;
-import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.execute.Execution;
 import org.apache.geode.cache.execute.Function;
@@ -54,6 +54,7 @@
 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.lang.StringUtils;
 import org.apache.geode.internal.util.IOUtils;
 import org.apache.geode.management.DistributedRegionMXBean;
 import org.apache.geode.management.ManagementService;
@@ -74,16 +75,18 @@
 public class CliUtil {
   public static final FileFilter JAR_FILE_FILTER = new CustomFileFilter(".jar");
 
-  public static InternalCache getCacheIfExists() {
-    InternalCache cache;
+  /**
+   * Returns the InternalCache returned by the provided method.
+   * If the provided method would raise a CacheClosedException, returns null instead.
+   */
+  public static InternalCache getCacheIfExists(Supplier<InternalCache> getCacheMethod) {
+    InternalCache cache = null;
     try {
-      cache = getInternalCache();
-    } catch (CacheClosedException e) {
-      // ignore & return null
-      cache = null;
+      cache = getCacheMethod.get();
+    } catch (CacheClosedException ignored) {
     }
-
     return cache;
+
   }
 
   public static String cliDependenciesExist(boolean includeGfshDependencies) {
@@ -212,7 +215,7 @@ public static Set<DistributedMember> getAllMembers(InternalDistributedSystem int
 
   public static Set<DistributedMember> getMembersWithAsyncEventQueue(InternalCache cache,
       String queueId) {
-    Set<DistributedMember> members = findMembers(null, null);
+    Set<DistributedMember> members = findMembers(null, null, cache);
     return members.stream().filter(m -> getAsyncEventQueueIds(cache, m).contains(queueId))
         .collect(Collectors.toSet());
   }
@@ -224,8 +227,7 @@ public static Set<String> getAsyncEventQueueIds(InternalCache cache, Distributed
         .map(x -> x.getKeyProperty("queue")).collect(Collectors.toSet());
   }
 
-  public static Set<String> getAllRegionNames() {
-    InternalCache cache = getInternalCache();
+  public static Set<String> getAllRegionNames(Cache cache) {
     Set<String> regionNames = new HashSet<>();
     Set<Region<?, ?>> rootRegions = cache.rootRegions();
 
@@ -246,19 +248,17 @@ public static Set<String> getAllRegionNames() {
    * groups or members.
    */
   public static Set<DistributedMember> findMembersIncludingLocators(String[] groups,
-      String[] members) {
-    InternalCache cache = getInternalCache();
+      String[] members, InternalCache cache) {
     Set<DistributedMember> allMembers = getAllMembers(cache);
-
     return findMembers(allMembers, groups, members);
   }
 
   /**
    * Finds all Servers which belong to the given arrays of groups or members. Does not include
    * locators.
    */
-  public static Set<DistributedMember> findMembers(String[] groups, String[] members) {
-    InternalCache cache = getInternalCache();
+  public static Set<DistributedMember> findMembers(String[] groups, String[] members,
+      InternalCache cache) {
     Set<DistributedMember> allNormalMembers = getAllNormalMembers(cache);
 
     return findMembers(allNormalMembers, groups, members);
@@ -303,23 +303,18 @@ private static Set<DistributedMember> findMembers(Set<DistributedMember> members
     return matchingMembers;
   }
 
-  public static DistributedMember getDistributedMemberByNameOrId(String memberNameOrId) {
-    DistributedMember memberFound = null;
-
-    if (memberNameOrId != null) {
-      InternalCache cache = getInternalCache();
-      Set<DistributedMember> memberSet = CliUtil.getAllMembers(cache);
-      for (DistributedMember member : memberSet) {
-        if (memberNameOrId.equalsIgnoreCase(member.getId())
-            || memberNameOrId.equalsIgnoreCase(member.getName())) {
-          memberFound = member;
-          break;
-        }
-      }
+  public static DistributedMember getDistributedMemberByNameOrId(String memberNameOrId,
+      InternalCache cache) {
+    if (memberNameOrId == null) {
+      return null;
     }
-    return memberFound;
+
+    Set<DistributedMember> memberSet = CliUtil.getAllMembers(cache);
+    return memberSet.stream().filter(member -> memberNameOrId.equalsIgnoreCase(member.getId())
+        || memberNameOrId.equalsIgnoreCase(member.getName())).findFirst().orElse(null);
   }
 
+
   /**
    * Even thought this is only used in a test, caller of MemberMXBean.processCommand(String, Map,
    * Byte[][]) will need to use this method to convert a fileList to Byte[][] to call that
@@ -566,10 +561,7 @@ public static void runLessCommandAsExternalViewer(Result commandResult) {
     }
   }
 
-
   static class CustomFileFilter implements FileFilter {
-
-
     private String extensionWithDot;
 
     public CustomFileFilter(String extensionWithDot) {
@@ -581,14 +573,12 @@ public boolean accept(File pathname) {
       String name = pathname.getName();
       return name.endsWith(extensionWithDot);
     }
-
   }
 
   public static class DeflaterInflaterData implements Serializable {
-
     private static final long serialVersionUID = 1104813333595216795L;
-    private final int dataLength;
 
+    private final int dataLength;
     private final byte[] data;
 
     public DeflaterInflaterData(int dataLength, byte[] data) {
@@ -609,10 +599,4 @@ public String toString() {
       return String.valueOf(dataLength);
     }
   }
-
-  // Methods that will be removed by the next commit:
-
-  private static InternalCache getInternalCache() {
-    return (InternalCache) CacheFactory.getAnyInstance();
-  }
 }

geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java

@@ -35,6 +35,7 @@
 import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
 import org.springframework.shell.core.annotation.CliCommand;
 
+import org.apache.geode.cache.Cache;
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.ClassPathLoader;
@@ -53,7 +54,6 @@ public class CommandManager {
   public static final String USER_CMD_PACKAGES_PROPERTY =
       DistributionConfig.GEMFIRE_PREFIX + USER_COMMAND_PACKAGES;
   public static final String USER_CMD_PACKAGES_ENV_VARIABLE = "GEMFIRE_USER_COMMAND_PACKAGES";
-  private static final Object INSTANCE_LOCK = new Object();
 
   private final Helper helper = new Helper();
 
@@ -68,18 +68,18 @@ public class CommandManager {
    * environment. used by Gfsh.
    */
   public CommandManager() {
-    this(null);
+    this(null, null);
   }
 
   /**
    * this is used when getting the instance in a cache server. We are getting the
    * user-command-package from distribution properties. used by OnlineCommandProcessor.
    */
-  public CommandManager(final Properties cacheProperties) {
+  public CommandManager(final Properties cacheProperties, Cache cache) {
     if (cacheProperties != null) {
       this.cacheProperties = cacheProperties;
     }
-    logWrapper = LogWrapper.getInstance();
+    logWrapper = LogWrapper.getInstance(cache);
     loadCommands();
   }
 
@@ -260,8 +260,6 @@ public List<CommandMarker> getCommandMarkers() {
 
   /**
    * Method to add new Converter
-   *
-   * @param converter
    */
   void add(Converter<?> converter) {
     if (CommandManagerAware.class.isAssignableFrom(converter.getClass())) {
@@ -272,8 +270,6 @@ void add(Converter<?> converter) {
 
   /**
    * Method to add new Commands to the parser
-   *
-   * @param commandMarker
    */
   void add(CommandMarker commandMarker) {
     if (CommandManagerAware.class.isAssignableFrom(commandMarker.getClass())) {

geode-core/src/main/java/org/apache/geode/management/internal/cli/Launcher.java

@@ -315,7 +315,8 @@ private static class StartupTimeLogHelper {
 
     public void logStartupTime() {
       done = System.currentTimeMillis();
-      LogWrapper.getInstance().info("Startup done in " + ((done - start) / 1000.0) + " seconds.");
+      LogWrapper.getInstance(null)
+          .info("Startup done in " + ((done - start) / 1000.0) + " seconds.");
     }
 
     @Override

geode-core/src/main/java/org/apache/geode/management/internal/cli/LogWrapper.java

@@ -45,10 +45,9 @@ public class LogWrapper {
 
   private Logger logger;
 
-  private LogWrapper() {
+  private LogWrapper(Cache cache) {
     logger = Logger.getLogger(this.getClass().getCanonicalName());
 
-    Cache cache = CliUtil.getCacheIfExists();
     if (cache != null && !cache.isClosed()) {
       logger.addHandler(cache.getLogger().getHandler());
       CommandResponseWriterHandler handler = new CommandResponseWriterHandler();
@@ -59,11 +58,11 @@ private LogWrapper() {
     logger.setUseParentHandlers(false);
   }
 
-  public static LogWrapper getInstance() {
+  public static LogWrapper getInstance(Cache cache) {
     if (INSTANCE == null) {
       synchronized (INSTANCE_LOCK) {
         if (INSTANCE == null) {
-          INSTANCE = new LogWrapper();
+          INSTANCE = new LogWrapper(cache);
         }
       }
     }

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterRuntimeConfigCommand.java

@@ -96,7 +96,7 @@ public Result alterRuntimeConfig(
 
     Map<String, String> runTimeDistributionConfigAttributes = new HashMap<>();
     Map<String, String> rumTimeCacheAttributes = new HashMap<>();
-    Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId);
+    Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId, getCache());
 
     if (targetMembers.isEmpty()) {
       return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ChangeLogLevelCommand.java

@@ -135,7 +135,8 @@ public Result changeLogLevel(
 
           }
         } catch (Exception ex) {
-          LogWrapper.getInstance().warning("change log level command exception " + ex);
+          LogWrapper.getInstance(CliUtil.getCacheIfExists(this::getCache))
+              .warning("change log level command exception " + ex);
         }
       }
 

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CloseDurableCQsCommand.java

@@ -57,7 +57,7 @@ public Result closeDurableCqs(@CliOption(key = CliStrings.CLOSE_DURABLE_CQS__DUR
           optionContext = ConverterHint.MEMBERGROUP) final String[] group) {
     Result result;
     try {
-      Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId);
+      Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId, getCache());
 
       if (targetMembers.isEmpty()) {
         return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CloseDurableClientCommand.java

@@ -55,7 +55,7 @@ public Result closeDurableClient(
     Result result;
     try {
 
-      Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId);
+      Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId, getCache());
 
       if (targetMembers.isEmpty()) {
         return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactDiskStoreCommand.java

@@ -122,7 +122,7 @@ public Result compactDiskStore(
             }
             String notExecutedMembers = CompactRequest.getNotExecutedMembers();
             if (notExecutedMembers != null && !notExecutedMembers.isEmpty()) {
-              LogWrapper.getInstance()
+              LogWrapper.getInstance(CliUtil.getCacheIfExists(this::getCache))
                   .info("compact disk-store \"" + diskStoreName
                       + "\" message was scheduled to be sent to but was not send to "
                       + notExecutedMembers);
@@ -159,7 +159,7 @@ public Result compactDiskStore(
         } // all members' if
       } // disk store exists' if
     } catch (RuntimeException e) {
-      LogWrapper.getInstance().info(e.getMessage(), e);
+      LogWrapper.getInstance(CliUtil.getCacheIfExists(this::getCache)).info(e.getMessage(), e);
       result = ResultBuilder.createGemFireErrorResult(
           CliStrings.format(CliStrings.COMPACT_DISK_STORE__ERROR_WHILE_COMPACTING_REASON_0,
               new Object[] {e.getMessage()}));

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java

@@ -54,7 +54,7 @@ public Result compactOfflineDiskStore(
       @CliOption(key = CliStrings.COMPACT_OFFLINE_DISK_STORE__J,
           help = CliStrings.COMPACT_OFFLINE_DISK_STORE__J__HELP) String[] jvmProps) {
     Result result;
-    LogWrapper logWrapper = LogWrapper.getInstance();
+    LogWrapper logWrapper = LogWrapper.getInstance(CliUtil.getCacheIfExists(this::getCache));
 
     StringBuilder output = new StringBuilder();
     StringBuilder error = new StringBuilder();

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigurePDXCommand.java

@@ -63,7 +63,7 @@ public Result configurePDX(
           && (patterns != null && patterns.length > 0)) {
         return ResultBuilder.createUserErrorResult(CliStrings.CONFIGURE_PDX__ERROR__MESSAGE);
       }
-      if (!CliUtil.getAllNormalMembers(CliUtil.getCacheIfExists()).isEmpty()) {
+      if (!CliUtil.getAllNormalMembers(getCache()).isEmpty()) {
         ird.addLine(CliStrings.CONFIGURE_PDX__NORMAL__MEMBERS__WARNING);
       }
       // Set persistent and the disk-store

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java

@@ -282,7 +282,7 @@ Result httpConnect(Properties gfProperties, String url, boolean skipSslVerificat
 
       gfsh.setOperationInvoker(operationInvoker);
 
-      LogWrapper.getInstance()
+      LogWrapper.getInstance(CliUtil.getCacheIfExists(this::getCache))
           .info(CliStrings.format(CliStrings.CONNECT__MSG__SUCCESS, operationInvoker.toString()));
       return ResultBuilder.createInfoResult(
           CliStrings.format(CliStrings.CONNECT__MSG__SUCCESS, operationInvoker.toString()));
@@ -358,8 +358,8 @@ Result jmxConnect(Properties gfProperties, boolean useSsl, ConnectionEndpoint me
       gfsh.setOperationInvoker(operationInvoker);
       infoResultData.addLine(CliStrings.format(CliStrings.CONNECT__MSG__SUCCESS,
           jmxHostPortToConnect.toString(false)));
-      LogWrapper.getInstance().info(CliStrings.format(CliStrings.CONNECT__MSG__SUCCESS,
-          jmxHostPortToConnect.toString(false)));
+      LogWrapper.getInstance(CliUtil.getCacheIfExists(this::getCache)).info(CliStrings
+          .format(CliStrings.CONNECT__MSG__SUCCESS, jmxHostPortToConnect.toString(false)));
       return ResultBuilder.buildResult(infoResultData);
     } catch (SecurityException | AuthenticationFailedException e) {
       // if it's security exception, and we already sent in username and password, still returns the
@@ -497,7 +497,7 @@ private Result handleException(Exception e) {
   }
 
   private Result handleException(Exception e, String errorMessage) {
-    LogWrapper.getInstance().severe(errorMessage, e);
+    LogWrapper.getInstance(CliUtil.getCacheIfExists(this::getCache)).severe(errorMessage, e);
     return ResultBuilder.createConnectionErrorResult(errorMessage);
   }
 

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CountDurableCQEventsCommand.java

@@ -56,7 +56,7 @@ public Result countDurableCqEvents(
 
     Result result;
     try {
-      Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId);
+      Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrId, getCache());
 
       if (targetMembers.isEmpty()) {
         return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);