Deprecate url methods and call in new location

solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java

@@ -18,13 +18,13 @@
 package org.apache.solr.client.solrj.impl;
 
 import static org.apache.solr.client.solrj.SolrClient.RemoteSolrException;
-import static org.apache.solr.common.util.Utils.getNodeNameFromSolrUrl;
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.time.Instant;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -47,6 +47,7 @@
 import org.apache.solr.common.util.EnvUtils;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.URLUtil;
 import org.apache.solr.common.util.Utils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -55,7 +56,7 @@ public abstract class BaseHttpClusterStateProvider implements ClusterStateProvid
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   private String urlScheme;
-  private Set<String> backupNodes;
+  private List<String> configuredNodes;
   volatile Set<String> liveNodes;
   long liveNodesTimestamp = 0;
   volatile Map<String, List<String>> aliases;
@@ -66,7 +67,7 @@ public abstract class BaseHttpClusterStateProvider implements ClusterStateProvid
   private int cacheTimeout = EnvUtils.getPropertyAsInteger("solr.solrj.cache.timeout.sec", 5);
 
   public void init(List<String> solrUrls) throws Exception {
-    this.backupNodes = getNodeNamesFromSolrUrls(solrUrls);
+    this.configuredNodes = getNodeNamesFromSolrUrls(solrUrls);
     for (String solrUrl : solrUrls) {
       urlScheme = solrUrl.startsWith("https") ? "https" : "http";
       try (SolrClient initialClient = getSolrClient(solrUrl)) {
@@ -86,7 +87,7 @@ public void init(List<String> solrUrls) throws Exception {
               + "succeeded in obtaining the cluster state from none of them."
               + "If you think your Solr cluster is up and is accessible,"
               + " you could try re-creating a new CloudSolrClient using working"
-              + " solrUrl(s) or zkHost(s).");
+              + " solrUrl(s).");
     }
   }
 
@@ -131,7 +132,7 @@ public ClusterState.CollectionRef getState(String collection) {
             + "succeeded in obtaining the cluster state from none of them."
             + "If you think your Solr cluster is up and is accessible,"
             + " you could try re-creating a new CloudSolrClient using working"
-            + " solrUrl(s) or zkHost(s).");
+            + " solrUrl(s).");
   }
 
   @SuppressWarnings("unchecked")
@@ -222,14 +223,6 @@ private SimpleOrderedMap<?> submitClusterStateRequest(
 
   @Override
   public Set<String> getLiveNodes() {
-    if (liveNodes == null) {
-      throw new RuntimeException(
-          "We don't know of any live_nodes to fetch the"
-              + " latest live_nodes information from. "
-              + "If you think your Solr cluster is up and is accessible,"
-              + " you could try re-creating a new CloudSolrClient using working"
-              + " solrUrl(s) or zkHost(s).");
-    }
     if (TimeUnit.SECONDS.convert((System.nanoTime() - liveNodesTimestamp), TimeUnit.NANOSECONDS)
         > getCacheTimeout()) {
 
@@ -239,7 +232,7 @@ > getCacheTimeout()) {
           "Attempt to fetch cluster state from all known live nodes {} failed. Trying backup nodes",
           liveNodes);
 
-      if (updateLiveNodes(backupNodes)) return this.liveNodes;
+      if (updateLiveNodes(configuredNodes)) return this.liveNodes;
 
       throw new RuntimeException(
           "Tried fetching live_nodes using all the node names we knew of, i.e. "
@@ -248,13 +241,13 @@ > getCacheTimeout()) {
               + "succeeded in obtaining the cluster state from none of them."
               + "If you think your Solr cluster is up and is accessible,"
               + " you could try re-creating a new CloudSolrClient using working"
-              + " solrUrl(s) or zkHost(s).");
+              + " solrUrl(s).");
     } else {
       return this.liveNodes; // cached copy is fresh enough
     }
   }
 
-  private boolean updateLiveNodes(Set<String> liveNodes) {
+  private boolean updateLiveNodes(Collection<String> liveNodes) {
     for (String nodeName : liveNodes) {
       String baseUrl = Utils.getBaseUrlForNodeName(nodeName, urlScheme);
       try (SolrClient client = getSolrClient(baseUrl)) {
@@ -296,7 +289,7 @@ private Map<String, List<String>> getAliases(boolean forceFetch) {
               + " latest aliases information from. "
               + "If you think your Solr cluster is up and is accessible,"
               + " you could try re-creating a new CloudSolrClient using working"
-              + " solrUrl(s) or zkHost(s).");
+              + " solrUrl(s).");
     }
 
     if (forceFetch
@@ -318,7 +311,7 @@ > getCacheTimeout()) {
           if (e instanceof RemoteSolrException && ((RemoteSolrException) e).code() == 400) {
             log.warn(
                 "LISTALIASES not found, possibly using older Solr server. Aliases won't work {}",
-                "unless you re-create the CloudSolrClient using zkHost(s) or upgrade Solr server",
+                "unless you upgrade Solr server",
                 e);
             this.aliases = Collections.emptyMap();
             this.aliasProperties = Collections.emptyMap();
@@ -336,7 +329,7 @@ > getCacheTimeout()) {
               + "succeeded in obtaining the cluster state from none of them."
               + "If you think your Solr cluster is up and is accessible,"
               + " you could try re-creating a new CloudSolrClient using a working"
-              + " solrUrl or zkHost.");
+              + " solrUrl.");
     } else {
       return Collections.unmodifiableMap(this.aliases); // cached copy is fresh enough
     }
@@ -370,7 +363,7 @@ public ClusterState getClusterState() {
             + "succeeded in obtaining the cluster state from none of them."
             + "If you think your Solr cluster is up and is accessible,"
             + " you could try re-creating a new CloudSolrClient using working"
-            + " solrUrl(s) or zkHost(s).");
+            + " solrUrl(s).");
   }
 
   @SuppressWarnings("unchecked")
@@ -393,7 +386,7 @@ public Map<String, Object> getClusterProperties() {
             + "succeeded in obtaining the cluster state from none of them."
             + "If you think your Solr cluster is up and is accessible,"
             + " you could try re-creating a new CloudSolrClient using working"
-            + " solrUrl(s) or zkHost(s).");
+            + " solrUrl(s).");
   }
 
   @Override
@@ -430,17 +423,17 @@ public String getQuorumHosts() {
     return String.join(",", this.liveNodes);
   }
 
-  public Set<String> getNodeNamesFromSolrUrls(List<String> urls) {
+  public List<String> getNodeNamesFromSolrUrls(List<String> urls) {
     return urls.stream()
         .map(
             (url) -> {
               try {
-                return getNodeNameFromSolrUrl(url);
+                return URLUtil.getNodeNameFromSolrUrl(url);
               } catch (MalformedURLException | URISyntaxException e) {
-                throw new RuntimeException("Failed to parse base Solr URL " + url, e);
+                throw new IllegalArgumentException("Failed to parse base Solr URL " + url, e);
               }
             })
-        .collect(Collectors.toUnmodifiableSet());
+        .collect(Collectors.toUnmodifiableList());
   }
 
   private enum ClusterStateRequestType {

solr/solrj/src/java/org/apache/solr/common/util/URLUtil.java

@@ -17,6 +17,10 @@
 package org.apache.solr.common.util;
 
 import java.lang.invoke.MethodHandles;
+import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.slf4j.Logger;
@@ -98,4 +102,60 @@ private static String removeTrailingSlashIfPresent(String url) {
 
     return url;
   }
+
+  /**
+   * Construct a V1 base url for the Solr node, given its name (e.g., 'app-node-1:8983_solr') and a
+   * URL scheme.
+   *
+   * @param nodeName name of the Solr node
+   * @param urlScheme scheme for the base url ('http' or 'https')
+   * @return url that looks like {@code https://app-node-1:8983/solr}
+   * @throws IllegalArgumentException if the provided node name is malformed
+   */
+  public static String getBaseUrlForNodeName(final String nodeName, final String urlScheme) {
+    return getBaseUrlForNodeName(nodeName, urlScheme, false);
+  }
+
+  /**
+   * Construct a V1 or a V2 base url for the Solr node, given its name (e.g.,
+   * 'app-node-1:8983_solr') and a URL scheme.
+   *
+   * @param nodeName name of the Solr node
+   * @param urlScheme scheme for the base url ('http' or 'https')
+   * @param isV2 whether a V2 url should be constructed
+   * @return url that looks like {@code https://app-node-1:8983/api} (V2) or {@code
+   *     https://app-node-1:8983/solr} (V1)
+   * @throws IllegalArgumentException if the provided node name is malformed
+   */
+  @Deprecated
+  public static String getBaseUrlForNodeName(
+      final String nodeName, final String urlScheme, boolean isV2) {
+    final int colonAt = nodeName.indexOf(':');
+    if (colonAt == -1) {
+      throw new IllegalArgumentException(
+          "nodeName does not contain expected ':' separator: " + nodeName);
+    }
+
+    final int _offset = nodeName.indexOf('_', colonAt);
+    if (_offset < 0) {
+      throw new IllegalArgumentException(
+          "nodeName does not contain expected '_' separator: " + nodeName);
+    }
+    final String hostAndPort = nodeName.substring(0, _offset);
+    return urlScheme + "://" + hostAndPort + "/" + (isV2 ? "api" : "solr");
+  }
+
+  /**
+   * Construct base Solr URL to a Solr node name
+   *
+   * @param solrUrl Given a base Solr URL string (e.g., 'https://app-node-1:8983/solr')
+   * @return URL that looks like {@code app-node-1:8983_solr}
+   * @throws MalformedURLException if the provided URL string is malformed
+   * @throws URISyntaxException if the provided URL string could not be parsed as a URI reference.
+   */
+  public static String getNodeNameFromSolrUrl(String solrUrl)
+      throws MalformedURLException, URISyntaxException {
+    URL url = new URI(solrUrl).toURL();
+    return url.getAuthority() + url.getPath().replace('/', '_');
+  }
 }

solr/solrj/src/java/org/apache/solr/common/util/Utils.java

@@ -40,9 +40,6 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.math.BigInteger;
-import java.net.MalformedURLException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.BufferOverflowException;
 import java.nio.ByteBuffer;
@@ -756,9 +753,11 @@ public static String applyUrlScheme(final String url, final String urlScheme) {
    * @param urlScheme scheme for the base url ('http' or 'https')
    * @return url that looks like {@code https://app-node-1:8983/solr}
    * @throws IllegalArgumentException if the provided node name is malformed
+   * @deprecated Use {@link URLUtil#getBaseUrlForNodeName(String, String)}
    */
+  @Deprecated
   public static String getBaseUrlForNodeName(final String nodeName, final String urlScheme) {
-    return getBaseUrlForNodeName(nodeName, urlScheme, false);
+    return URLUtil.getBaseUrlForNodeName(nodeName, urlScheme, false);
   }
 
   /**
@@ -771,36 +770,12 @@ public static String getBaseUrlForNodeName(final String nodeName, final String u
    * @return url that looks like {@code https://app-node-1:8983/api} (V2) or {@code
    *     https://app-node-1:8983/solr} (V1)
    * @throws IllegalArgumentException if the provided node name is malformed
+   * @deprecated Use {@link URLUtil#getBaseUrlForNodeName(String, String, boolean)}
    */
+  @Deprecated
   public static String getBaseUrlForNodeName(
       final String nodeName, final String urlScheme, boolean isV2) {
-    final int colonAt = nodeName.indexOf(':');
-    if (colonAt == -1) {
-      throw new IllegalArgumentException(
-          "nodeName does not contain expected ':' separator: " + nodeName);
-    }
-
-    final int _offset = nodeName.indexOf('_', colonAt);
-    if (_offset < 0) {
-      throw new IllegalArgumentException(
-          "nodeName does not contain expected '_' separator: " + nodeName);
-    }
-    final String hostAndPort = nodeName.substring(0, _offset);
-    return urlScheme + "://" + hostAndPort + "/" + (isV2 ? "api" : "solr");
-  }
-
-  /**
-   * Construct base Solr URL to a Solr node name
-   *
-   * @param solrUrl Given a base Solr URL string (e.g., 'https://app-node-1:8983/solr')
-   * @return URL that looks like {@code app-node-1:8983_solr}
-   * @throws MalformedURLException if the provided URL string is malformed
-   * @throws URISyntaxException if the provided URL string could not be parsed as a URI reference.
-   */
-  public static String getNodeNameFromSolrUrl(String solrUrl)
-      throws MalformedURLException, URISyntaxException {
-    URL url = new URI(solrUrl).toURL();
-    return url.getAuthority() + url.getPath().replace('/', '_');
+    return URLUtil.getBaseUrlForNodeName(nodeName, urlScheme, isV2);
   }
 
   public static long time(TimeSource timeSource, TimeUnit unit) {

solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java

@@ -17,7 +17,7 @@
 
 package org.apache.solr.client.solrj.impl;
 
-import static org.apache.solr.common.util.Utils.getNodeNameFromSolrUrl;
+import static org.apache.solr.common.util.URLUtil.getNodeNameFromSolrUrl;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 

solr/solrj/src/test/org/apache/solr/common/util/URLUtilTest.java

@@ -16,6 +16,11 @@
  */
 package org.apache.solr.common.util;
 
+import static org.apache.solr.common.util.URLUtil.getNodeNameFromSolrUrl;
+import static org.apache.solr.common.util.Utils.getBaseUrlForNodeName;
+
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
 import org.apache.solr.SolrTestCase;
 import org.junit.Test;
 
@@ -95,4 +100,29 @@ public void testCanBuildCoreUrl() {
         "http://localhost:8983/solr/sTrAnGe-name.for_core",
         URLUtil.buildCoreUrl("http://localhost:8983/solr", "sTrAnGe-name.for_core"));
   }
+
+  @Test
+  public void testGetNodeNameFromSolrUrl() throws MalformedURLException, URISyntaxException {
+    assertEquals("node-1-url:8983_solr", getNodeNameFromSolrUrl("https://node-1-url:8983/solr"));
+    assertEquals("node-1-url:8983_solr", getNodeNameFromSolrUrl("http://node-1-url:8983/solr"));
+    assertEquals("node-1-url:8983_api", getNodeNameFromSolrUrl("http://node-1-url:8983/api"));
+    assertThrows(MalformedURLException.class, () -> getNodeNameFromSolrUrl("node-1-url:8983/solr"));
+    assertThrows(
+        URISyntaxException.class, () -> getNodeNameFromSolrUrl("http://node-1-url:8983/solr^"));
+  }
+
+  @Test
+  public void testGetBaseUrlForNodeName() {
+    assertEquals(
+        "http://app-node-1:8983/solr",
+        URLUtil.getBaseUrlForNodeName("app-node-1:8983_solr", "http", false));
+    assertEquals(
+        "https://app-node-1:8983/solr",
+        getBaseUrlForNodeName("app-node-1:8983_solr", "https", false));
+    assertEquals(
+        "http://app-node-1:8983/api", getBaseUrlForNodeName("app-node-1:8983_solr", "http", true));
+    assertEquals(
+        "https://app-node-1:8983/api",
+        getBaseUrlForNodeName("app-node-1:8983_solr", "https", true));
+  }
 }