Forward port PR#2710 (apache#2726)

* Simplify method calls, and forward port changes from PR#2710
jdyer1 · Sep 29, 2024 · 2f0fe1f · 2f0fe1f
1 parent 9c44a47
commit 2f0fe1f
Show file tree

Hide file tree

Showing 15 changed files with 71 additions and 64 deletions.
diff --git a/solr/core/src/java/org/apache/solr/cli/AuthTool.java b/solr/core/src/java/org/apache/solr/cli/AuthTool.java
@@ -265,7 +265,7 @@ private int handleKerberos(CommandLine cli) throws Exception {
 
         if (!updateIncludeFileOnly) {
           if (!zkInaccessible) {
-            echoIfVerbose("Uploading following security.json: " + securityJson, cli);
+            echoIfVerbose("Uploading following security.json: " + securityJson);
             try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
               zkClient.setData(
                   "/security.json", securityJson.getBytes(StandardCharsets.UTF_8), true);
@@ -424,7 +424,7 @@ private int handleBasicAuth(CommandLine cli) throws Exception {
         String securityJson = securityJson1.toPrettyString();
 
         if (!updateIncludeFileOnly) {
-          echoIfVerbose("Uploading following security.json: " + securityJson, cli);
+          echoIfVerbose("Uploading following security.json: " + securityJson);
           try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
             zkClient.setData("/security.json", securityJson.getBytes(StandardCharsets.UTF_8), true);
           }
@@ -514,7 +514,7 @@ private void clearSecurityJson(CommandLine cli, boolean updateIncludeFileOnly) t
         SolrCLI.exit(1);
       }
 
-      echoIfVerbose("Uploading following security.json: {}", cli);
+      echoIfVerbose("Uploading following security.json: {}");
 
       try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
         zkClient.setData("/security.json", "{}".getBytes(StandardCharsets.UTF_8), true);
@@ -624,9 +624,9 @@ private void updateIncludeFileEnableAuth(
     Files.writeString(includeFile, lines, StandardCharsets.UTF_8);
 
     if (basicAuthConfFile != null) {
-      echoIfVerbose("Written out credentials file: " + basicAuthConfFile, cli);
+      echoIfVerbose("Written out credentials file: " + basicAuthConfFile);
     }
-    echoIfVerbose("Updated Solr include file: " + includeFile.toAbsolutePath(), cli);
+    echoIfVerbose("Updated Solr include file: " + includeFile.toAbsolutePath());
   }
 
   private void updateIncludeFileDisableAuth(Path includeFile, CommandLine cli) throws IOException {
@@ -648,7 +648,7 @@ private void updateIncludeFileDisableAuth(Path includeFile, CommandLine cli) thr
     if (hasChanged) {
       String lines = includeFileLines.stream().collect(Collectors.joining(System.lineSeparator()));
       Files.writeString(includeFile, lines, StandardCharsets.UTF_8);
-      echoIfVerbose("Commented out necessary lines from " + includeFile.toAbsolutePath(), cli);
+      echoIfVerbose("Commented out necessary lines from " + includeFile.toAbsolutePath());
     }
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java
@@ -107,7 +107,7 @@ public void runImpl(CommandLine cli) throws Exception {
     String zkHost = SolrCLI.getZkHost(cli);
 
     try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
-      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
       String confName =
           cli.hasOption("conf-name")
               ? cli.getOptionValue("conf-name")

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java
@@ -118,7 +118,7 @@ public void runImpl(CommandLine cli) throws Exception {
     String confDir =
         cli.hasOption("conf-dir") ? cli.getOptionValue("conf-dir") : cli.getOptionValue("confdir");
     try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
-      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
 
       final Path configsetsDirPath = SolrCLI.getConfigSetsDir(solrInstallDirPath);
       Path confPath = ConfigSetService.getConfigsetPath(confDir, configsetsDirPath.toString());

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigTool.java
@@ -107,7 +107,7 @@ public List<Option> getOptions() {
             .longOpt("value")
             .argName("VALUE")
             .hasArg()
-            .required(false) // should be true when -v is removed.
+            .required(false)
             .desc("Set the property to this value; accepts JSON objects and strings.")
             .build(),
         SolrCLI.OPTION_SOLRURL,
@@ -128,7 +128,9 @@ public void runImpl(CommandLine cli) throws Exception {
     if (property == null) {
       throw new MissingArgumentException("'property' is a required option.");
     }
-    if (value == null) {
+
+    // value is required unless the property is one of the unset- type.
+    if (!action.contains("unset-") && value == null) {
       throw new MissingArgumentException("'value' is a required option.");
     }
     Map<String, Object> jsonObj = new HashMap<>();
@@ -147,7 +149,7 @@ public void runImpl(CommandLine cli) throws Exception {
     String updatePath = "/" + collection + "/config";
 
     echo("\nPOSTing request to Config API: " + solrUrl + updatePath);
-    echoIfVerbose(jsonBody, cli);
+    echoIfVerbose(jsonBody);
 
     try (SolrClient solrClient =
         SolrCLI.getSolrClient(

diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java
@@ -226,20 +226,19 @@ protected void createCore(CommandLine cli, SolrClient solrClient) throws Excepti
 
       echoIfVerbose(
           "\nCopying configuration to new core instance directory:\n"
-              + coreInstanceDir.toAbsolutePath(),
-          cli);
+              + coreInstanceDir.toAbsolutePath());
     }
 
-    echoIfVerbose("\nCreating new core '" + coreName + "' using CoreAdminRequest", cli);
+    echoIfVerbose("\nCreating new core '" + coreName + "' using CoreAdminRequest");
 
     try {
       CoreAdminResponse res = CoreAdminRequest.createCore(coreName, coreName, solrClient);
-      if (verbose) {
+      if (isVerbose()) {
         echo(res.jsonStr());
         echo("\n");
-      } else {
-        echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
       }
+      echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
+
     } catch (Exception e) {
       /* create-core failed, cleanup the copied configset before propagating the error. */
       PathUtils.deleteDirectory(coreInstanceDir);
@@ -256,8 +255,8 @@ protected void createCollection(CommandLine cli) throws Exception {
             .withOptionalBasicAuthCredentials(
                 cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()));
     String zkHost = SolrCLI.getZkHost(cli);
+    echoIfVerbose("Connecting to ZooKeeper at " + zkHost);
     try (CloudSolrClient cloudSolrClient = SolrCLI.getCloudHttp2SolrClient(zkHost, builder)) {
-      echoIfVerbose("Connecting to ZooKeeper at " + zkHost, cli);
       cloudSolrClient.connect();
       createCollection(cloudSolrClient, cli);
     }
@@ -331,8 +330,7 @@ protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine cli
               + " for config "
               + confName
               + " to ZooKeeper at "
-              + cloudSolrClient.getClusterStateProvider().getQuorumHosts(),
-          cli);
+              + cloudSolrClient.getClusterStateProvider().getQuorumHosts());
       // We will trust the config since we have the Zookeeper Address
       configSetService.uploadConfig(confName, confPath, true);
     }
@@ -348,7 +346,7 @@ protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine cli
 
     // doesn't seem to exist ... try to create
     echoIfVerbose(
-        "\nCreating new collection '" + collectionName + "' using CollectionAdminRequest", cli);
+        "\nCreating new collection '" + collectionName + "' using CollectionAdminRequest");
 
     NamedList<Object> response;
     try {
@@ -362,25 +360,24 @@ protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine cli
           "Failed to create collection '" + collectionName + "' due to: " + sse.getMessage());
     }
 
-    if (verbose) {
+    if (isVerbose()) {
       // pretty-print the response to stdout
       CharArr arr = new CharArr();
       new JSONWriter(arr, 2).write(response.asMap());
       echo(arr.toString());
-    } else {
-      String endMessage =
-          String.format(
-              Locale.ROOT,
-              "Created collection '%s' with %d shard(s), %d replica(s)",
-              collectionName,
-              numShards,
-              replicationFactor);
-      if (confName != null && !confName.trim().isEmpty()) {
-        endMessage += String.format(Locale.ROOT, " with config-set '%s'", confName);
-      }
-
-      echo(endMessage);
     }
+    String endMessage =
+        String.format(
+            Locale.ROOT,
+            "Created collection '%s' with %d shard(s), %d replica(s)",
+            collectionName,
+            numShards,
+            replicationFactor);
+    if (confName != null && !confName.trim().isEmpty()) {
+      endMessage += String.format(Locale.ROOT, " with config-set '%s'", confName);
+    }
+
+    echo(endMessage);
   }
 
   private Path getFullConfDir(Path solrInstallDir, Path confDirName) {

diff --git a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
@@ -21,6 +21,7 @@
 import java.io.PrintStream;
 import java.lang.invoke.MethodHandles;
 import java.util.List;
+import java.util.Locale;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -149,7 +150,7 @@ protected void deleteCollection(CommandLine cli) throws Exception {
 
     String zkHost = SolrCLI.getZkHost(cli);
     try (CloudSolrClient cloudSolrClient = SolrCLI.getCloudHttp2SolrClient(zkHost, builder)) {
-      echoIfVerbose("Connecting to ZooKeeper at " + zkHost, cli);
+      echoIfVerbose("Connecting to ZooKeeper at " + zkHost);
       cloudSolrClient.connect();
       deleteCollection(cloudSolrClient, cli);
     }
@@ -215,8 +216,7 @@ protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli
       }
     }
 
-    echoIfVerbose(
-        "\nDeleting collection '" + collectionName + "' using CollectionAdminRequest", cli);
+    echoIfVerbose("\nDeleting collection '" + collectionName + "' using CollectionAdminRequest");
 
     NamedList<Object> response;
     try {
@@ -238,19 +238,19 @@ protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli
                 + configZnode
                 + " in ZooKeeper due to: "
                 + exc.getMessage()
-                + "\nYou'll need to manually delete this znode using the zkcli script.");
+                + "\nYou'll need to manually delete this znode using the bin/solr zk rm command.");
       }
     }
 
-    if (response != null) {
+    if (isVerbose() && response != null) {
       // pretty-print the response to stdout
       CharArr arr = new CharArr();
       new JSONWriter(arr, 2).write(response.asMap());
       echo(arr.toString());
       echo("\n");
     }
 
-    echo("Deleted collection '" + collectionName + "' using CollectionAdminRequest");
+    echo(String.format(Locale.ROOT, "\nDeleted collection '%s'", collectionName));
   }
 
   protected void deleteCore(CommandLine cli, SolrClient solrClient) throws Exception {
@@ -272,8 +272,8 @@ protected void deleteCore(CommandLine cli, SolrClient solrClient) throws Excepti
     }
 
     if (response != null) {
-      echoIfVerbose((String) response.get("response"), cli);
-      echoIfVerbose("\n", cli);
+      echoIfVerbose((String) response.get("response"));
+      echoIfVerbose("\n");
     }
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java
@@ -95,7 +95,7 @@ public void runImpl(CommandLine cli) throws Exception {
       System.exit(1);
     }
     try (CloudHttp2SolrClient cloudSolrClient = SolrCLI.getCloudHttp2SolrClient(zkHost)) {
-      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
       cloudSolrClient.connect();
       runCloudTool(cloudSolrClient, cli);
     }

diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java
@@ -240,8 +240,7 @@ public void runImpl(CommandLine cli) throws Exception {
             + ",\nexampleDir="
             + exampleDir.getAbsolutePath()
             + "\nscript="
-            + script,
-        cli);
+            + script);
 
     String exampleType = cli.getOptionValue("example");
     if ("cloud".equals(exampleType)) {
@@ -497,7 +496,7 @@ protected void runCloudExample(CommandLine cli) throws Exception {
         }
 
         cloudPorts[n] = port;
-        echoIfVerbose("Using port " + port + " for node " + (n + 1), cli);
+        echoIfVerbose("Using port " + port + " for node " + (n + 1));
       }
     } else {
       echo("Starting up " + numNodes + " Solr nodes for your example SolrCloud cluster.\n");
@@ -708,7 +707,7 @@ protected Map<String, Object> startSolr(
     if (code != 0) throw new Exception("Failed to start Solr using command: " + startCmd);
 
     return getNodeStatus(
-        solrUrl, cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()), maxWaitSecs, cli);
+        solrUrl, cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()), maxWaitSecs);
   }
 
   protected Map<String, Object> checkPortConflict(
@@ -887,10 +886,10 @@ protected boolean isValidConfig(File configsetsDir, String config) {
     return configDir.isDirectory();
   }
 
-  protected Map<String, Object> getNodeStatus(
-      String solrUrl, String credentials, int maxWaitSecs, CommandLine cli) throws Exception {
+  protected Map<String, Object> getNodeStatus(String solrUrl, String credentials, int maxWaitSecs)
+      throws Exception {
     StatusTool statusTool = new StatusTool();
-    echoIfVerbose("\nChecking status of Solr at " + solrUrl + " ...", cli);
+    echoIfVerbose("\nChecking status of Solr at " + solrUrl + " ...");
 
     URI solrURI = new URI(solrUrl);
     Map<String, Object> nodeStatus =
@@ -901,8 +900,7 @@ protected Map<String, Object> getNodeStatus(
     String mode = (nodeStatus.get("cloud") != null) ? "cloud" : "standalone";
 
     echoIfVerbose(
-        "\nSolr is running on " + solrURI.getPort() + " in " + mode + " mode with status:\n" + arr,
-        cli);
+        "\nSolr is running on " + solrURI.getPort() + " in " + mode + " mode with status:\n" + arr);
 
     return nodeStatus;
   }

diff --git a/solr/core/src/java/org/apache/solr/cli/ToolBase.java b/solr/core/src/java/org/apache/solr/cli/ToolBase.java
@@ -33,8 +33,13 @@ protected ToolBase(PrintStream stdout) {
     this.stdout = stdout;
   }
 
-  protected void echoIfVerbose(final String msg, CommandLine cli) {
-    if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+  /** Is this tool being run in a verbose mode? */
+  protected boolean isVerbose() {
+    return verbose;
+  }
+
+  protected void echoIfVerbose(final String msg) {
+    if (verbose) {
       echo(msg);
     }
   }

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkCpTool.java b/solr/core/src/java/org/apache/solr/cli/ZkCpTool.java
@@ -128,7 +128,7 @@ public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
     String zkHost = SolrCLI.getZkHost(cli);
 
-    echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+    echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
     String src = cli.getArgs()[0];
     String dst = cli.getArgs()[1];
     boolean recurse = cli.hasOption("recurse");
@@ -166,7 +166,7 @@ public void runImpl(CommandLine cli) throws Exception {
       }
 
       if (solrHome != null) {
-        echoIfVerbose("Using SolrHome: " + solrHome, cli);
+        echoIfVerbose("Using SolrHome: " + solrHome);
         try {
           // Be aware that if you start Solr and pass in some variables via -D like
           // solr start -DminStateByteLenForCompression=0 -c, this logic will not
@@ -199,7 +199,7 @@ public void runImpl(CommandLine cli) throws Exception {
       }
     }
     if (minStateByteLenForCompression > -1) {
-      echoIfVerbose("Compression of state.json has been enabled", cli);
+      echoIfVerbose("Compression of state.json has been enabled");
     }
     try (SolrZkClient zkClient =
         new SolrZkClient.Builder()

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkLsTool.java b/solr/core/src/java/org/apache/solr/cli/ZkLsTool.java
@@ -66,7 +66,7 @@ public void runImpl(CommandLine cli) throws Exception {
     String znode = cli.getArgs()[0];
 
     try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
-      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
 
       boolean recurse = cli.hasOption("recurse");
       echoIfVerbose(
@@ -75,8 +75,7 @@ public void runImpl(CommandLine cli) throws Exception {
               + " from ZooKeeper at "
               + zkHost
               + " recurse: "
-              + recurse,
-          cli);
+              + recurse);
       stdout.print(zkClient.listZnode(znode, recurse));
     } catch (Exception e) {
       log.error("Could not complete ls operation for reason: ", e);

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkMkrootTool.java b/solr/core/src/java/org/apache/solr/cli/ZkMkrootTool.java
@@ -83,7 +83,7 @@ public void runImpl(CommandLine cli) throws Exception {
     boolean failOnExists = cli.hasOption("fail-on-exists");
 
     try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
-      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
 
       echo("Creating ZooKeeper path " + znode + " on ZooKeeper at " + zkHost);
       zkClient.makePath(znode, failOnExists, true);

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkMvTool.java b/solr/core/src/java/org/apache/solr/cli/ZkMvTool.java
@@ -82,7 +82,7 @@ public void runImpl(CommandLine cli) throws Exception {
     String zkHost = SolrCLI.getZkHost(cli);
 
     try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
-      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
       String src = cli.getArgs()[0];
       String dst = cli.getArgs()[1];
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkRmTool.java b/solr/core/src/java/org/apache/solr/cli/ZkRmTool.java
@@ -75,7 +75,7 @@ public void runImpl(CommandLine cli) throws Exception {
     if (znode.equals("/")) {
       throw new SolrServerException("You may not remove the root ZK node ('/')!");
     }
-    echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
+    echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...");
     try (SolrZkClient zkClient = SolrCLI.getSolrZkClient(cli, zkHost)) {
       if (!recurse && zkClient.getChildren(znode, null, true).size() != 0) {
         throw new SolrServerException(