You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ep...@apache.org on 2024/05/23 19:21:49 UTC
(solr) branch branch_9x updated: SOLR-16659: Properly construct V2 base urls instead of replacing substring "/solr" with "/api" (#2473)
This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 4ed9f273817 SOLR-16659: Properly construct V2 base urls instead of replacing substring "/solr" with "/api" (#2473)
4ed9f273817 is described below
commit 4ed9f2738171d8a6a580e603913a3d3686a3aa9c
Author: Andrey Bozhko <an...@gmail.com>
AuthorDate: Thu May 23 14:20:56 2024 -0500
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.
---
solr/CHANGES.txt | 2 ++
.../apache/solr/filestore/DistribFileStore.java | 22 +++++++++----------
.../src/java/org/apache/solr/pkg/PackageAPI.java | 12 ++---------
.../apache/solr/cloud/AliasIntegrationTest.java | 7 +++---
.../org/apache/solr/cloud/MigrateReplicasTest.java | 4 ++--
.../org/apache/solr/cloud/TestConfigSetsAPI.java | 10 ++++-----
.../solr/filestore/TestDistribFileStore.java | 4 ++--
.../apache/solr/handler/TestContainerPlugin.java | 2 +-
.../solr/handler/admin/ZookeeperReadAPITest.java | 6 ++++--
.../src/test/org/apache/solr/pkg/TestPackages.java | 3 +--
.../solr/security/AuditLoggerIntegrationTest.java | 5 ++---
.../apache/solr/common/cloud/ZkStateReader.java | 25 ++++++++++++++++------
.../solr/client/solrj/impl/HttpSolrClient.java | 2 +-
.../java/org/apache/solr/common/util/Utils.java | 20 +++++++++++++++++
14 files changed, 73 insertions(+), 51 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 6d38129719f..6b52c2d62ff 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -46,6 +46,8 @@ Bug Fixes
* 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 3cf8cbca3a3..4c62c1ff20e 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 @@ public class DistribFileStore implements FileStore {
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 @@ public class DistribFileStore implements FileStore {
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 @@ public class DistribFileStore implements FileStore {
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 @@ public class DistribFileStore implements FileStore {
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 6e264421684..7e2c5a6f73e 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 @@ public class PackageAPI {
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 @@ public class PackageAPI {
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 c3a9e77f55d..3863043fc1a 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 class AliasIntegrationTest extends SolrCloudTestCase {
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 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
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 0be4ca1746b..f96b621c023 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 @@ public class MigrateReplicasTest extends SolrCloudTestCase {
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 57a067cc217..a45232a3af9 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 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
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 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
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 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
final ByteBuffer sampleConfigFile =
TestSolrConfigHandler.getFileContent(file.getAbsolutePath(), false);
final String uriEnding =
- "/api/cluster/configs/"
+ "/cluster/configs/"
+ configSetName
+ suffix
+ "/"
@@ -1646,8 +1645,7 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
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 a6fa83b579c..62668789d22 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 @@ public class TestDistribFileStore extends SolrCloudTestCase {
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 @@ public class TestDistribFileStore extends SolrCloudTestCase {
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 e6b4c98ccb4..faa07395241 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
@@ -573,7 +573,7 @@ public class TestContainerPlugin extends SolrCloudTestCase {
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 e77290dc230..39d494e177c 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 @@ public class ZookeeperReadAPITest extends SolrCloudTestCase {
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 d70aa372737..aae36019a65 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 @@ public class TestPackages extends SolrCloudTestCase {
// 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 41e3788110c..f349488a82a 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 @@ public class AuditLoggerIntegrationTest extends SolrCloudAuthTestCase {
@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 c57fcb03400..7d5cb5bae64 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
@@ -120,7 +120,7 @@ public class ZkStateReader implements SolrCloseable {
/**
* 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";
@@ -161,7 +161,6 @@ public class ZkStateReader implements SolrCloseable {
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";
@@ -1329,10 +1328,24 @@ public class ZkStateReader implements SolrCloseable {
* 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. */
@@ -2263,7 +2276,7 @@ public class ZkStateReader implements SolrCloseable {
}
/**
- * 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
*/
@@ -2271,7 +2284,7 @@ public class ZkStateReader implements SolrCloseable {
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 e8f4beadd0f..ae8fc5e899c 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
@@ -387,7 +387,7 @@ public class HttpSolrClient extends BaseHttpSolrClient {
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 52639ce648f..04ea727099e 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
@@ -717,10 +717,30 @@ public class Utils {
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(':');