SOLR-16659: Properly construct V2 base urls instead of replacing substring "/solr" with "/api" (#2473)
Refactoring and clean up on how to craft URLS that are V2 from V1 sources.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d672ae9..56ca1a3 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -139,6 +139,8 @@
* PR-2475: Fixed node listing bug in Admin UI when different hostnames start with the same front part. (@hgdharold via Eric Pugh)
+* SOLR-16659: Properly construct V2 base urls instead of replacing substring "/solr" with "/api" (Andrey Bozhko via Eric Pugh)
+
Dependency Upgrades
---------------------
(No changes)
diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java
index 3cf8cbc..4c62c1f 100644
--- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java
+++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java
@@ -175,10 +175,9 @@
private boolean fetchFileFromNodeAndPersist(String fromNode) {
log.info("fetching a file {} from {} ", path, fromNode);
- String url =
- coreContainer.getZkController().getZkStateReader().getBaseUrlForNodeName(fromNode);
- if (url == null) throw new SolrException(BAD_REQUEST, "No such node");
- String baseUrl = url.replace("/solr", "/api");
+ String baseUrl =
+ coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(fromNode);
+ if (baseUrl == null) throw new SolrException(BAD_REQUEST, "No such node");
ByteBuffer metadata = null;
Map<?, ?> m = null;
@@ -220,10 +219,9 @@
ArrayList<String> l = coreContainer.getFileStoreAPI().shuffledNodes();
for (String liveNode : l) {
try {
- String baseurl =
- coreContainer.getZkController().getZkStateReader().getBaseUrlForNodeName(liveNode);
- String url = baseurl.replace("/solr", "/api");
- String reqUrl = url + "/node/files" + path + "?meta=true&wt=javabin&omitHeader=true";
+ String baseUrl =
+ coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(liveNode);
+ String reqUrl = baseUrl + "/node/files" + path + "?meta=true&wt=javabin&omitHeader=true";
boolean nodeHasBlob = false;
Object nl =
Utils.executeGET(
@@ -362,8 +360,8 @@
try {
for (String node : nodes) {
String baseUrl =
- coreContainer.getZkController().getZkStateReader().getBaseUrlForNodeName(node);
- String url = baseUrl.replace("/solr", "/api") + "/node/files" + info.path + "?getFrom=";
+ coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(node);
+ String url = baseUrl + "/node/files" + info.path + "?getFrom=";
if (i < FETCHFROM_SRC) {
// this is to protect very large clusters from overwhelming a single node
// the first FETCHFROM_SRC nodes will be asked to fetch from this node.
@@ -502,8 +500,8 @@
HttpClient client = coreContainer.getUpdateShardHandler().getDefaultHttpClient();
for (String node : nodes) {
String baseUrl =
- coreContainer.getZkController().getZkStateReader().getBaseUrlForNodeName(node);
- String url = baseUrl.replace("/solr", "/api") + "/node/files" + path;
+ coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(node);
+ String url = baseUrl + "/node/files" + path;
HttpDelete del = new HttpDelete(url);
// invoke delete command on all nodes asynchronously
coreContainer.runAsync(() -> Utils.executeHttpMethod(client, url, null, del));
diff --git a/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java b/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java
index 6e26442..7e2c5a6 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java
@@ -257,11 +257,7 @@
for (String s : coreContainer.getFileStoreAPI().shuffledNodes()) {
Utils.executeGET(
coreContainer.getUpdateShardHandler().getDefaultHttpClient(),
- coreContainer
- .getZkController()
- .zkStateReader
- .getBaseUrlForNodeName(s)
- .replace("/solr", "/api")
+ coreContainer.getZkController().zkStateReader.getBaseUrlV2ForNodeName(s)
+ "/cluster/package?wt=javabin&omitHeader=true&refreshPackage="
+ p,
Utils.JAVABINCONSUMER);
@@ -429,11 +425,7 @@
for (String s : coreContainer.getFileStoreAPI().shuffledNodes()) {
Utils.executeGET(
coreContainer.getUpdateShardHandler().getDefaultHttpClient(),
- coreContainer
- .getZkController()
- .zkStateReader
- .getBaseUrlForNodeName(s)
- .replace("/solr", "/api")
+ coreContainer.getZkController().zkStateReader.getBaseUrlV2ForNodeName(s)
+ "/cluster/package?wt=javabin&omitHeader=true&expectedVersion"
+ expected,
Utils.JAVABINCONSUMER);
diff --git a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
index c3a9e77..3863043 100644
--- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
@@ -238,9 +238,8 @@
public void testModifyPropertiesV2() throws Exception {
final String aliasName = getSaferTestName();
ZkStateReader zkStateReader = createColectionsAndAlias(aliasName);
- final String baseUrl =
- cluster.getRandomJetty(random()).getBaseUrl().toString().replace("/solr", "");
- String aliasApi = String.format(Locale.ENGLISH, "/api/aliases/%s/properties", aliasName);
+ final String baseUrl = cluster.getRandomJetty(random()).getBaseURLV2().toString();
+ String aliasApi = String.format(Locale.ENGLISH, "/aliases/%s/properties", aliasName);
HttpPut withoutBody = new HttpPut(baseUrl + aliasApi);
assertEquals(400, httpClient.execute(withoutBody).getStatusLine().getStatusCode());
@@ -260,7 +259,7 @@
checkFooAndBarMeta(aliasName, zkStateReader, "baz", "bam");
String aliasPropertyApi =
- String.format(Locale.ENGLISH, "/api/aliases/%s/properties/%s", aliasName, "foo");
+ String.format(Locale.ENGLISH, "/aliases/%s/properties/%s", aliasName, "foo");
HttpPut updateByProperty = new HttpPut(baseUrl + aliasPropertyApi);
updateByProperty.setEntity(
new StringEntity("{ \"value\": \"zab\" }", ContentType.APPLICATION_JSON));
diff --git a/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java b/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java
index 0be4ca1..f96b621 100644
--- a/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java
@@ -344,8 +344,8 @@
Map<?, ?> r = null;
String uri =
- cluster.getJettySolrRunners().get(0).getBaseUrl().toString().replace("/solr", "")
- + "/api/cluster/replicas/migrate";
+ cluster.getJettySolrRunners().get(0).getBaseURLV2().toString()
+ + "/cluster/replicas/migrate";
try {
httpRequest = new HttpPost(uri);
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
index 57a067c..a45232a 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
@@ -1581,7 +1581,7 @@
final ByteBuffer fileBytes =
TestSolrConfigHandler.getFileContent(file.getAbsolutePath(), false);
final String uriEnding =
- "/api/cluster/configs/"
+ "/cluster/configs/"
+ configSetName
+ suffix
+ (!overwrite ? "?overwrite=false" : "")
@@ -1590,8 +1590,7 @@
Map<?, ?> map =
postDataAndGetResponse(
cluster.getSolrClient(),
- cluster.getJettySolrRunners().get(0).getBaseUrl().toString().replace("/solr", "")
- + uriEnding,
+ cluster.getJettySolrRunners().get(0).getBaseURLV2().toString() + uriEnding,
fileBytes,
username,
usePut);
@@ -1634,7 +1633,7 @@
final ByteBuffer sampleConfigFile =
TestSolrConfigHandler.getFileContent(file.getAbsolutePath(), false);
final String uriEnding =
- "/api/cluster/configs/"
+ "/cluster/configs/"
+ configSetName
+ suffix
+ "/"
@@ -1646,8 +1645,7 @@
Map<?, ?> map =
postDataAndGetResponse(
cluster.getSolrClient(),
- cluster.getJettySolrRunners().get(0).getBaseUrl().toString().replace("/solr", "")
- + uriEnding,
+ cluster.getJettySolrRunners().get(0).getBaseURLV2().toString() + uriEnding,
sampleConfigFile,
username,
usePut);
diff --git a/solr/core/src/test/org/apache/solr/filestore/TestDistribFileStore.java b/solr/core/src/test/org/apache/solr/filestore/TestDistribFileStore.java
index a6fa83b..6266878 100644
--- a/solr/core/src/test/org/apache/solr/filestore/TestDistribFileStore.java
+++ b/solr/core/src/test/org/apache/solr/filestore/TestDistribFileStore.java
@@ -169,7 +169,7 @@
return true;
});
for (JettySolrRunner jettySolrRunner : cluster.getJettySolrRunners()) {
- String baseUrl = jettySolrRunner.getBaseUrl().toString().replace("/solr", "/api");
+ String baseUrl = jettySolrRunner.getBaseURLV2().toString();
String url = baseUrl + "/node/files/package/mypkg/v1.0?wt=javabin";
assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
}
@@ -196,7 +196,7 @@
boolean verifyContent)
throws Exception {
for (JettySolrRunner jettySolrRunner : cluster.getJettySolrRunners()) {
- String baseUrl = jettySolrRunner.getBaseUrl().toString().replace("/solr", "/api");
+ String baseUrl = jettySolrRunner.getBaseURLV2().toString();
String url = baseUrl + "/node/files" + path + "?wt=javabin&meta=true";
assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
diff --git a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
index eabadb8..bd19c67 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
@@ -572,7 +572,7 @@
public void waitForAllNodesToSync(String path, Map<String, Object> expected) throws Exception {
for (JettySolrRunner jettySolrRunner : cluster.getJettySolrRunners()) {
- String baseUrl = jettySolrRunner.getBaseUrl().toString().replace("/solr", "/api");
+ String baseUrl = jettySolrRunner.getBaseURLV2().toString();
String url = baseUrl + path + "?wt=javabin";
TestDistribFileStore.assertResponseValues(1, new Fetcher(url, jettySolrRunner), expected);
}
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java
index e77290d..39d494e 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/ZookeeperReadAPITest.java
@@ -52,8 +52,10 @@
super.setUp();
baseUrl = cluster.getJettySolrRunner(0).getBaseUrl();
- basezk = baseUrl.toString().replace("/solr", "/api") + "/cluster/zookeeper/data";
- basezkls = baseUrl.toString().replace("/solr", "/api") + "/cluster/zookeeper/children";
+
+ String baseUrlV2 = cluster.getJettySolrRunner(0).getBaseURLV2().toString();
+ basezk = baseUrlV2 + "/cluster/zookeeper/data";
+ basezkls = baseUrlV2 + "/cluster/zookeeper/children";
}
@After
diff --git a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
index d70aa37..aae3601 100644
--- a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
+++ b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
@@ -691,8 +691,7 @@
// So far we have been verifying the details with ZK directly
// use the package read API to verify with each node that it has the correct data
for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
- String path =
- jetty.getBaseUrl().toString().replace("/solr", "/api") + "/cluster/package?wt=javabin";
+ String path = jetty.getBaseURLV2().toString() + "/cluster/package?wt=javabin";
TestDistribFileStore.assertResponseValues(
10,
new Callable<NavigableObject>() {
diff --git a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
index 2232ddb..47886c2 100644
--- a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
@@ -253,12 +253,11 @@
@Test
public void illegalAdminPathError() throws Exception {
setupCluster(false, null, false);
- String baseUrl = testHarness.get().cluster.getJettySolrRunner(0).getBaseUrl().toString();
+ String baseUrl = testHarness.get().cluster.getJettySolrRunner(0).getBaseURLV2().toString();
expectThrows(
FileNotFoundException.class,
() -> {
- try (InputStream is =
- new URL(baseUrl.replace("/solr", "") + "/api/node/foo").openStream()) {
+ try (InputStream is = new URL(baseUrl + "/node/foo").openStream()) {
new String(is.readAllBytes(), StandardCharsets.UTF_8);
}
});
diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 32ec0b2..b9882dd 100644
--- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -119,7 +119,7 @@
/**
* This ZooKeeper file is no longer used starting with Solr 9 but keeping the name around to check
- * if it is still present and non empty (in case of upgrade from previous Solr version). It used
+ * if it is still present and non-empty (in case of upgrade from previous Solr version). It used
* to contain collection state for all collections in the cluster.
*/
public static final String UNSUPPORTED_CLUSTER_STATE = "/clusterstate.json";
@@ -163,7 +163,6 @@
private static final int GET_LEADER_RETRY_INTERVAL_MS = 50;
private static final int GET_LEADER_RETRY_DEFAULT_TIMEOUT =
Integer.parseInt(System.getProperty("zkReaderGetLeaderRetryTimeoutMs", "4000"));
- ;
public static final String LEADER_ELECT_ZKNODE = "leader_elect";
@@ -1331,10 +1330,24 @@
* Returns the baseURL corresponding to a given node's nodeName -- NOTE: does not (currently)
* imply that the nodeName (or resulting baseURL) exists in the cluster.
*
- * @lucene.experimental
+ * @param nodeName name of the node
+ * @return url that looks like {@code https://localhost:8983/solr}
*/
public String getBaseUrlForNodeName(final String nodeName) {
- return Utils.getBaseUrlForNodeName(nodeName, getClusterProperty(URL_SCHEME, "http"));
+ String urlScheme = getClusterProperty(URL_SCHEME, "http");
+ return Utils.getBaseUrlForNodeName(nodeName, urlScheme, false);
+ }
+
+ /**
+ * Returns the V2 baseURL corresponding to a given node's nodeName -- NOTE: does not (currently)
+ * imply that the nodeName (or resulting baseURL) exists in the cluster.
+ *
+ * @param nodeName name of the node
+ * @return url that looks like {@code https://localhost:8983/api}
+ */
+ public String getBaseUrlV2ForNodeName(final String nodeName) {
+ String urlScheme = getClusterProperty(URL_SCHEME, "http");
+ return Utils.getBaseUrlForNodeName(nodeName, urlScheme, true);
}
/** Watches a single collection's state.json. */
@@ -2265,7 +2278,7 @@
}
/**
- * Ensures the internal aliases is up to date. If there is a change, return true.
+ * Ensures the internal aliases is up-to-date. If there is a change, return true.
*
* @return true if an update was performed
*/
@@ -2273,7 +2286,7 @@
if (log.isDebugEnabled()) {
log.debug("Checking ZK for most up to date Aliases {}", ALIASES);
}
- // Call sync() first to ensure the subsequent read (getData) is up to date.
+ // Call sync() first to ensure the subsequent read (getData) is up-to-date.
zkClient.getZooKeeper().sync(ALIASES, null, null);
return setIfNewer(zkClient.getNode(ALIASES, null, true));
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
index 612d95b..b2a08a7 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
@@ -374,7 +374,7 @@
if (request instanceof V2Request) {
if (System.getProperty("solr.v2RealPath") == null || ((V2Request) request).isForceV2()) {
- basePath = baseUrl.replace("/solr", "/api");
+ basePath = changeV2RequestEndpoint(baseUrl);
} else {
basePath = baseUrl + "/____v2";
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 4a1b39b..f37038e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -716,10 +716,30 @@
return (at == -1) ? (urlScheme + "://" + url) : urlScheme + url.substring(at);
}
+ /**
+ * 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
+ */
public static String getBaseUrlForNodeName(
final String nodeName, final String urlScheme, boolean isV2) {
final int colonAt = nodeName.indexOf(':');
