SOLR-17431: Deprecate -p parameter where it doesn't refer to a port. (#…

…2690)

solr/CHANGES.txt

@@ -119,6 +119,8 @@ Improvements
 
 * SOLR-17382: Deprecate -a and -addlopts in favour of --jvm-opts for passing options into the JVM in bin/solr.   (Eric Pugh, Christos Malliaridis)
 
+* SOLR-17431: Deprecate -p parameter where it doesn't refer to a port in bin/solr. (Eric Pugh, Christos Malliaridis)
+
 Optimizations
 ---------------------
 * SOLR-14985: Solrj CloudSolrClient with Solr URLs had serious performance regressions (since the

solr/core/src/java/org/apache/solr/cli/ConfigTool.java

@@ -22,6 +22,8 @@
 import java.util.List;
 import java.util.Map;
 import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.DeprecatedAttributes;
+import org.apache.commons.cli.MissingArgumentException;
 import org.apache.commons.cli.Option;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.common.util.NamedList;
@@ -66,11 +68,26 @@ public List<Option> getOptions() {
             .desc(
                 "Config API action, one of: set-property, unset-property, set-user-property, unset-user-property; default is 'set-property'.")
             .build(),
-        Option.builder("p")
+        Option.builder()
             .longOpt("property")
             .argName("PROP")
             .hasArg()
-            .required(true)
+            .required(
+                false) // Should be TRUE but have a deprecated option to deal with first, so we
+            // enforce in code
+            .desc(
+                "Name of the Config API property to apply the action to, such as: 'updateHandler.autoSoftCommit.maxTime'.")
+            .build(),
+        Option.builder("p")
+            .deprecated(
+                DeprecatedAttributes.builder()
+                    .setForRemoval(true)
+                    .setSince("9.8")
+                    .setDescription("Use --property instead")
+                    .get())
+            .hasArg()
+            .argName("PROP")
+            .required(false)
             .desc(
                 "Name of the Config API property to apply the action to, such as: 'updateHandler.autoSoftCommit.maxTime'.")
             .build(),
@@ -93,9 +110,13 @@ public void runImpl(CommandLine cli) throws Exception {
     String solrUrl = SolrCLI.normalizeSolrUrl(cli);
     String action = cli.getOptionValue("action", "set-property");
     String collection = cli.getOptionValue("name");
-    String property = cli.getOptionValue("property");
+    String property = SolrCLI.getOptionWithDeprecatedAndDefault(cli, "property", "p", null);
     String value = cli.getOptionValue("value");
 
+    if (property == null) {
+      throw new MissingArgumentException("'property' is a required option.");
+    }
+
     Map<String, Object> jsonObj = new HashMap<>();
     if (value != null) {
       Map<String, String> setMap = new HashMap<>();

solr/core/src/java/org/apache/solr/cli/PackageTool.java

@@ -27,6 +27,7 @@
 import java.util.List;
 import java.util.Map;
 import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.DeprecatedAttributes;
 import org.apache.commons.cli.Option;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.config.Configurator;
@@ -124,7 +125,7 @@ public void runImpl(CommandLine cli) throws Exception {
                   printGreen("\t" + packages.get(packageName));
                 }
               } else {
-                // nuance that we use a arg here instead of requiring a --package parameter with a
+                // nuance that we use an arg here instead of requiring a --package parameter with a
                 // value
                 // in this code path
                 String packageName = cli.getArgs()[1];
@@ -166,21 +167,25 @@ public void runImpl(CommandLine cli) throws Exception {
                   Pair<String, String> parsedVersion = parsePackageVersion(cli.getArgList().get(1));
                   String packageName = parsedVersion.first();
                   String version = parsedVersion.second();
-                  boolean noprompt = cli.hasOption("no-prompt");
+                  boolean noPrompt = cli.hasOption("no-prompt");
                   boolean isUpdate = cli.hasOption("update");
                   String[] collections =
                       cli.hasOption("collections")
                           ? PackageUtils.validateCollections(
                               cli.getOptionValue("collections").split(","))
                           : new String[] {};
+                  String[] parameters =
+                      cli.hasOption("param")
+                          ? cli.getOptionValues("param")
+                          : cli.getOptionValues("p");
                   packageManager.deploy(
                       packageName,
                       version,
                       collections,
                       cli.hasOption("cluster"),
-                      cli.getOptionValues("param"),
+                      parameters,
                       isUpdate,
-                      noprompt);
+                      noPrompt);
                 } else {
                   printRed(
                       "Either specify --cluster to deploy cluster level plugins or --collections <list-of-collections> to deploy collection level plugins");
@@ -330,12 +335,24 @@ public List<Option> getOptions() {
             .longOpt("cluster")
             .desc("Specifies that this action should affect cluster-level plugins only.")
             .build(),
-        Option.builder("p")
+        Option.builder()
             .longOpt("param")
             .hasArgs()
             .argName("PARAMS")
             .desc("List of parameters to be used with deploy command.")
             .build(),
+        Option.builder("p")
+            .deprecated(
+                DeprecatedAttributes.builder()
+                    .setForRemoval(true)
+                    .setSince("9.8")
+                    .setDescription("Use --param instead")
+                    .get())
+            .hasArg()
+            .argName("PARAMS")
+            .required(false)
+            .desc("List of parameters to be used with deploy command.")
+            .build(),
         Option.builder()
             .longOpt("update")
             .desc("If a deployment is an update over a previous deployment.")

solr/core/src/java/org/apache/solr/cli/PostTool.java

@@ -243,12 +243,24 @@ public List<Option> getOptions() {
             .required(false)
             .desc("default: " + DEFAULT_FILE_TYPES)
             .build(),
-        Option.builder("p")
+        Option.builder()
             .longOpt("params")
             .hasArg()
             .argName("<key>=<value>[&<key>=<value>...]")
             .required(false)
-            .desc("values must be URL-encoded; these pass through to Solr update request.")
+            .desc("Values must be URL-encoded; these pass through to Solr update request.")
+            .build(),
+        Option.builder("p")
+            .deprecated(
+                DeprecatedAttributes.builder()
+                    .setForRemoval(true)
+                    .setSince("9.8")
+                    .setDescription("Use --params instead")
+                    .get())
+            .hasArg()
+            .argName("<key>=<value>[&<key>=<value>...]")
+            .required(false)
+            .desc("Values must be URL-encoded; these pass through to Solr update request.")
             .build(),
         Option.builder()
             .longOpt("out")
@@ -313,7 +325,7 @@ public void runImpl(CommandLine cli) throws Exception {
 
     args = cli.getArgs();
 
-    params = cli.getOptionValue("params", "");
+    params = SolrCLI.getOptionWithDeprecatedAndDefault(cli, "params", "p", "");
 
     execute(mode);
   }

solr/core/src/java/org/apache/solr/cli/SolrCLI.java

@@ -371,6 +371,19 @@ public static Options getToolOptions(Tool tool) {
     return options;
   }
 
+  /**
+   * Returns the value of the option with the given name, or the value of the deprecated option. If
+   * both values are null, then it returns the default value.
+   */
+  public static String getOptionWithDeprecatedAndDefault(
+      CommandLine cli, String opt, String deprecated, String def) {
+    String val = cli.getOptionValue(opt);
+    if (val == null) {
+      val = cli.getOptionValue(deprecated);
+    }
+    return val == null ? def : val;
+  }
+
   // TODO: SOLR-17429 - remove the custom logic when CommonsCLI is upgraded and
   // makes stderr the default, or makes Option.toDeprecatedString() public.
   private static void deprecatedHandlerStdErr(Option o) {

solr/core/src/test/org/apache/solr/cli/PackageToolTest.java

@@ -157,6 +157,7 @@ public void testPackageTool() throws Exception {
           SecurityJson.USER_PASS
         });
 
+    // Leaving -p in for --params to test the deprecated value continues to work.
     run(
         tool,
         new String[] {
@@ -263,7 +264,7 @@ public void testPackageTool() throws Exception {
               "question-answer",
               "--collections",
               "abc",
-              "-p",
+              "--params",
               "RH-HANDLER-PATH=" + rhPath,
               "--credentials",
               SecurityJson.USER_PASS

solr/solr-ref-guide/modules/configuration-guide/pages/package-manager.adoc

@@ -102,7 +102,7 @@ There are two ways to do this: either use the CLI's `deploy` command or manually
 If the package author states support for it, the package can be deployed with the CLI's `deploy` command.
 [source,bash]
 ----
-$ bin/solr package deploy <package-name>:[version] --collections <collection1>[,<collection2>,...] [-p <param1>=<val1> -p <param2>=<val2> ...
+$ bin/solr package deploy <package-name>:[version] --collections <collection1>[,<collection2>,...] [--param <param1>=<val1> --param <param2>=<val2> ...
 ----
 
 or
@@ -116,7 +116,7 @@ If the package accepts parameters for its setup commands, they can be specified
 
 [source,bash]
 ----
-$ bin/solr package deploy <snipped...> -p <param1>=<val1> -p <param2>=<val2>
+$ bin/solr package deploy <snipped...> --param <param1>=<val1> --param <param2>=<val2>
 ----
 
 The author may want you to confirm deployment of a package via a prompt.

solr/solr-ref-guide/modules/indexing-guide/pages/post-tool.adoc

@@ -74,7 +74,7 @@ usage: post
                                                   of posting documents.
     --out                                         sends Solr response
                                                   outputs to console.
- -p,--params <<key>=<value>[&<key>=<value>...]>   values must be
+ --params <<key>=<value>[&<key>=<value>...]>      values must be
                                                   URL-encoded; these pass
                                                   through to Solr update
                                                   request.