SOLR-16800: Move towards using the root of Solr as the -solrUrl in th…

…e CLI. (#1808) Now that hostContext can only ever be /solr, and the maturing of the Solr V2 API's that are nested under the /api path, we want Solr commands that take in a -solrUrl to point to the root of your Solr, not a nested path. This handles this, as well as provides feedback to users who still use the older -solrUrl http://localhost:8983/solr form that they no longer need to do this.
apache · Aug 8, 2023 · 44668dc · 44668dc
1 parent aa9f867
commit 44668dc
Show file tree

Hide file tree

Showing 21 changed files with 219 additions and 120 deletions.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -15,6 +15,8 @@ Improvements
 
 * SOLR-16850: Notify a user if they run healthcheck against standalone Solr that it is a SolrCloud only command. (Eric Pugh)
 
+* SOLR-16800: Solr CLI commands that use -solrUrl now work with a bare url like http://localhost:8983, you no longer need to add the /solr at the end. (Eric Pugh)
+
 Optimizations
 ---------------------
 (No changes)

diff --git a/solr/core/src/java/org/apache/solr/cli/AssertTool.java b/solr/core/src/java/org/apache/solr/cli/AssertTool.java
@@ -29,7 +29,6 @@
 import org.apache.commons.cli.Option;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.client.solrj.impl.Http2SolrClient;
 import org.apache.solr.client.solrj.request.HealthCheckRequest;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.NamedList;
@@ -198,10 +197,10 @@ protected int runAssert(CommandLine cli) throws Exception {
       ret += assertSolrNotRunning(cli.getOptionValue("S"));
     }
     if (cli.hasOption("c")) {
-      ret += assertSolrRunningInCloudMode(cli.getOptionValue("c"));
+      ret += assertSolrRunningInCloudMode(SolrCLI.normalizeSolrUrl(cli.getOptionValue("c")));
     }
     if (cli.hasOption("C")) {
-      ret += assertSolrNotRunningInCloudMode(cli.getOptionValue("C"));
+      ret += assertSolrNotRunningInCloudMode(SolrCLI.normalizeSolrUrl(cli.getOptionValue("C")));
     }
     return ret;
   }
@@ -348,11 +347,11 @@ public static String userForDir(Path pathToDir) {
     }
   }
 
-  private static int exitOrException(String msg) throws SolrCLI.AssertionFailureException {
+  private static int exitOrException(String msg) throws AssertionFailureException {
     if (useExitCode) {
       return 1;
     } else {
-      throw new SolrCLI.AssertionFailureException(message != null ? message : msg);
+      throw new AssertionFailureException(message != null ? message : msg);
     }
   }
 
@@ -370,8 +369,14 @@ private static boolean isSolrRunningOn(String url) throws Exception {
   }
 
   private static boolean runningSolrIsCloud(String url) throws Exception {
-    try (final SolrClient client = new Http2SolrClient.Builder(url).build()) {
+    try (final SolrClient client = SolrCLI.getSolrClient(url)) {
       return SolrCLI.isCloudMode(client);
     }
   }
+
+  public static class AssertionFailureException extends Exception {
+    public AssertionFailureException(String message) {
+      super(message);
+    }
+  }
 }
diff --git a/solr/core/src/java/org/apache/solr/cli/AuthTool.java b/solr/core/src/java/org/apache/solr/cli/AuthTool.java
@@ -111,12 +111,8 @@ public List<Option> getOptions() {
             .desc(
                 "This is where any authentication related configuration files, if any, would be placed.")
             .build(),
-        Option.builder("solrUrl").argName("solrUrl").hasArg().desc("Solr URL.").build(),
-        Option.builder("zkHost")
-            .argName("zkHost")
-            .hasArg()
-            .desc("ZooKeeper host to connect to.")
-            .build(),
+        SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_ZKHOST,
         SolrCLI.OPTION_VERBOSE);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigTool.java
@@ -24,7 +24,6 @@
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.solr.client.solrj.SolrClient;
-import org.apache.solr.client.solrj.impl.Http2SolrClient;
 import org.apache.solr.common.util.NamedList;
 import org.noggit.CharArr;
 import org.noggit.JSONWriter;
@@ -85,7 +84,7 @@ public List<Option> getOptions() {
 
   @Override
   public void runImpl(CommandLine cli) throws Exception {
-    String solrUrl = SolrCLI.resolveSolrUrl(cli);
+    String solrUrl = SolrCLI.normalizeSolrUrl(cli);
     String action = cli.getOptionValue("action", "set-property");
     String collection = cli.getOptionValue("name");
     String property = cli.getOptionValue("property");
@@ -109,7 +108,7 @@ public void runImpl(CommandLine cli) throws Exception {
     echo("\nPOSTing request to Config API: " + solrUrl + updatePath);
     echo(jsonBody);
 
-    try (SolrClient solrClient = new Http2SolrClient.Builder(solrUrl).build()) {
+    try (SolrClient solrClient = SolrCLI.getSolrClient(solrUrl)) {
       NamedList<Object> result = SolrCLI.postJsonToSolr(solrClient, updatePath, jsonBody);
       Integer statusCode = (Integer) result.findRecursive("responseHeader", "status");
       if (statusCode == 0) {

diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java
@@ -117,7 +117,7 @@ public List<Option> getOptions() {
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
-    String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
+    String solrUrl = SolrCLI.normalizeSolrUrl(cli);
 
     try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
       if (SolrCLI.isCloudMode(solrClient)) {

diff --git a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
@@ -88,7 +88,7 @@ public List<Option> getOptions() {
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
-    String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
+    String solrUrl = SolrCLI.normalizeSolrUrl(cli);
 
     try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
       if (SolrCLI.isCloudMode(solrClient)) {

diff --git a/solr/core/src/java/org/apache/solr/cli/ExportTool.java b/solr/core/src/java/org/apache/solr/cli/ExportTool.java
@@ -94,7 +94,39 @@ public String getName() {
 
   @Override
   public List<Option> getOptions() {
-    return OPTIONS;
+    return List.of(
+        Option.builder("url")
+            .hasArg()
+            .required()
+            .desc("Address of the collection, example http://localhost:8983/solr/gettingstarted.")
+            .build(),
+        Option.builder("out")
+            .hasArg()
+            .required(false)
+            .desc(
+                "Path to output the exported data, and optionally the file name, defaults to 'collection-name'.")
+            .build(),
+        Option.builder("format")
+            .hasArg()
+            .required(false)
+            .desc("Output format for exported docs (json, jsonl or javabin), defaulting to json.")
+            .build(),
+        Option.builder("compress").required(false).desc("Compress the output.").build(),
+        Option.builder("limit")
+            .hasArg()
+            .required(false)
+            .desc("Maximum number of docs to download. Default is 100, use -1 for all docs.")
+            .build(),
+        Option.builder("query")
+            .hasArg()
+            .required(false)
+            .desc("A custom query, default is '*:*'.")
+            .build(),
+        Option.builder("fields")
+            .hasArg()
+            .required(false)
+            .desc("Comma separated list of fields to export. By default all fields are fetched.")
+            .build());
   }
 
   public abstract static class Info {
@@ -230,41 +262,6 @@ void accept(SolrDocument document) throws IOException {
     void end() throws IOException {}
   }
 
-  private static final List<Option> OPTIONS =
-      List.of(
-          Option.builder("url")
-              .hasArg()
-              .required()
-              .desc("Address of the collection, example http://localhost:8983/solr/gettingstarted.")
-              .build(),
-          Option.builder("out")
-              .hasArg()
-              .required(false)
-              .desc(
-                  "Path to output the exported data, and optionally the file name, defaults to 'collection-name'.")
-              .build(),
-          Option.builder("format")
-              .hasArg()
-              .required(false)
-              .desc("Output format for exported docs (json, jsonl or javabin), defaulting to json.")
-              .build(),
-          Option.builder("compress").required(false).desc("Compress the output.").build(),
-          Option.builder("limit")
-              .hasArg()
-              .required(false)
-              .desc("Maximum number of docs to download. Default is 100, use -1 for all docs.")
-              .build(),
-          Option.builder("query")
-              .hasArg()
-              .required(false)
-              .desc("A custom query, default is '*:*'.")
-              .build(),
-          Option.builder("fields")
-              .hasArg()
-              .required(false)
-              .desc("Comma separated list of fields to export. By default all fields are fetched.")
-              .build());
-
   static class JsonWithLinesSink extends DocsSink {
     private final CharArr charArr = new CharArr(1024 * 2);
     JSONWriter jsonWriter = new JSONWriter(charArr, -1);

diff --git a/solr/core/src/java/org/apache/solr/cli/PackageTool.java b/solr/core/src/java/org/apache/solr/cli/PackageTool.java
@@ -60,8 +60,6 @@ public String getName() {
     return "package";
   }
 
-  public static String solrUrl = null;
-  public static String solrBaseUrl = null;
   public PackageManager packageManager;
   public RepositoryManager repositoryManager;
 
@@ -79,19 +77,16 @@ public void runImpl(CommandLine cli) throws Exception {
         printHelp();
         return;
       }
-
-      solrUrl = cli.getOptionValues("solrUrl")[cli.getOptionValues("solrUrl").length - 1];
-      solrBaseUrl = solrUrl.replaceAll("/solr$", ""); // strip out ending "/solr"
-      log.info("Solr url:{}, solr base url: {}", solrUrl, solrBaseUrl);
+      String solrUrl = SolrCLI.normalizeSolrUrl(cli);
       String zkHost = SolrCLI.getZkHost(cli);
       if (zkHost == null) {
         throw new SolrException(ErrorCode.INVALID_STATE, "Package manager runs only in SolrCloud");
       }
 
       log.info("ZK: {}", zkHost);
 
-      try (SolrClient solrClient = new Http2SolrClient.Builder(solrBaseUrl).build()) {
-        packageManager = new PackageManager(solrClient, solrBaseUrl, zkHost);
+      try (SolrClient solrClient = new Http2SolrClient.Builder(solrUrl).build()) {
+        packageManager = new PackageManager(solrClient, solrUrl, zkHost);
         try {
           repositoryManager = new RepositoryManager(solrClient, packageManager);
 
@@ -242,7 +237,7 @@ public void runImpl(CommandLine cli) throws Exception {
 
     } catch (Exception ex) {
       // We need to print this since SolrCLI drops the stack trace in favour
-      // of brevity. Package tool should surely print full stacktraces!
+      // of brevity. Package tool should surely print the full stacktrace!
       ex.printStackTrace();
       throw ex;
     }
@@ -314,15 +309,7 @@ private Pair<String, String> parsePackageVersion(String arg) {
   @Override
   public List<Option> getOptions() {
     return List.of(
-        Option.builder("solrUrl")
-            .argName("URL")
-            .hasArg()
-            .required(true)
-            .desc(
-                "Address of the Solr Web application, defaults to: "
-                    + SolrCLI.DEFAULT_SOLR_URL
-                    + '.')
-            .build(),
+        SolrCLI.OPTION_SOLRURL,
         Option.builder("collections")
             .argName("COLLECTIONS")
             .hasArg()

diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java
@@ -400,7 +400,7 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception
                 + "        }\n");
 
         File filmsJsonFile = new File(exampleDir, "films/films.json");
-        String updateUrl = String.format(Locale.ROOT, "%s/%s/update/json", solrUrl, collectionName);
+        String updateUrl = String.format(Locale.ROOT, "%s/%s/update", solrUrl, collectionName);
         echo("Indexing films example docs from " + filmsJsonFile.getAbsolutePath());
         String[] args =
             new String[] {
@@ -410,6 +410,8 @@ protected void runExample(CommandLine cli, String exampleName) throws Exception
               updateUrl,
               "-type",
               "application/json",
+              "-filetypes",
+              "json",
               exampleDir.toString()
             };
         PostTool postTool = new PostTool();

diff --git a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
@@ -75,7 +75,7 @@ public class SolrCLI implements CLIO {
       TimeUnit.NANOSECONDS.convert(1, TimeUnit.MINUTES);
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-  public static final String DEFAULT_SOLR_URL = "http://localhost:8983/solr";
+  public static final String DEFAULT_SOLR_URL = "http://localhost:8983";
   public static final String ZK_HOST = "localhost:9983";
 
   public static final Option OPTION_ZKHOST =
@@ -86,7 +86,8 @@ public class SolrCLI implements CLIO {
           .required(false)
           .desc(
               "Zookeeper connection string; unnecessary if ZK_HOST is defined in solr.in.sh; otherwise, defaults to "
-                  + ZK_HOST)
+                  + ZK_HOST
+                  + '.')
           .longOpt("zkHost")
           .build();
   public static final Option OPTION_SOLRURL =
@@ -96,7 +97,8 @@ public class SolrCLI implements CLIO {
           .required(false)
           .desc(
               "Base Solr URL, which can be used to determine the zkHost if that's not known; defaults to: "
-                  + DEFAULT_SOLR_URL)
+                  + DEFAULT_SOLR_URL
+                  + '.')
           .build();
   public static final Option OPTION_VERBOSE =
       Option.builder("verbose").required(false).desc("Enable more verbose command output.").build();
@@ -382,6 +384,13 @@ public static boolean exceptionIsAuthRelated(Exception exc) {
   }
 
   public static SolrClient getSolrClient(String solrUrl) {
+    // today we require all urls to end in /solr, however in the future we will need to support the
+    // /api url end point instead.
+    // The /solr/ check is because sometimes a full url is passed in, like
+    // http://localhost:8983/solr/films_shard1_replica_n1/.
+    if (!solrUrl.endsWith("/solr") && !solrUrl.contains("/solr/")) {
+      solrUrl = solrUrl + "/solr";
+    }
     return new Http2SolrClient.Builder(solrUrl).withMaxConnectionsPerHost(32).build();
   }
 
@@ -444,11 +453,37 @@ private static void printHelp() {
     print("Pass -help or -h after any COMMAND to see command-specific usage information,");
     print("such as:    ./solr start -help or ./solr stop -h");
   }
+
   /**
-   * Get the base URL of a live Solr instance from either the solrUrl command-line option from
+   * Strips off the end of solrUrl any /solr when a legacy solrUrl like http://localhost:8983/solr
+   * is used, and warns those users. In the future we'll have url's with /api as well.
+   *
+   * @param solrUrl The user supplied url to Solr.
+   * @return the solrUrl in the format that Solr expects to see internally.
+   */
+  public static String normalizeSolrUrl(String solrUrl) {
+    if (solrUrl != null) {
+      if (solrUrl.indexOf("/solr") > -1) { //
+        String newSolrUrl = solrUrl.substring(0, solrUrl.indexOf("/solr"));
+        CLIO.out(
+            "WARNING: URLs provided to this tool needn't include Solr's context-root (e.g. \"/solr\"). Such URLs are deprecated and support for them will be removed in a future release. Correcting from ["
+                + solrUrl
+                + "] to ["
+                + newSolrUrl
+                + "].");
+        solrUrl = newSolrUrl;
+      }
+      if (solrUrl.endsWith("/")) {
+        solrUrl = solrUrl.substring(0, solrUrl.length() - 1);
+      }
+    }
+    return solrUrl;
+  }
+  /**
+   * Get the base URL of a live Solr instance from either the solrUrl command-line option or from
    * ZooKeeper.
    */
-  public static String resolveSolrUrl(CommandLine cli) throws Exception {
+  public static String normalizeSolrUrl(CommandLine cli) throws Exception {
     String solrUrl = cli.getOptionValue("solrUrl");
     if (solrUrl == null) {
       String zkHost = cli.getOptionValue("zkHost");
@@ -475,6 +510,7 @@ public static String resolveSolrUrl(CommandLine cli) throws Exception {
         }
       }
     }
+    solrUrl = normalizeSolrUrl(solrUrl);
     return solrUrl;
   }
 
@@ -488,7 +524,6 @@ public static String getZkHost(CommandLine cli) throws Exception {
       return zkHost;
     }
 
-    // find it using the localPort
     String solrUrl = cli.getOptionValue("solrUrl");
     if (solrUrl == null) {
       solrUrl = DEFAULT_SOLR_URL;
@@ -498,6 +533,7 @@ public static String getZkHost(CommandLine cli) throws Exception {
                   + DEFAULT_SOLR_URL
                   + ".");
     }
+    solrUrl = normalizeSolrUrl(solrUrl);
 
     try (var solrClient = getSolrClient(solrUrl)) {
       // hit Solr to get system info
@@ -568,10 +604,4 @@ public static boolean isCloudMode(SolrClient solrClient) throws SolrServerExcept
         solrClient.request(new GenericSolrRequest(SolrRequest.METHOD.GET, SYSTEM_INFO_PATH));
     return "solrcloud".equals(systemInfo.get("mode"));
   }
-
-  public static class AssertionFailureException extends Exception {
-    public AssertionFailureException(String message) {
-      super(message);
-    }
-  }
 }