SOLR-17153: Fix IndexOutOfBoundsException, add bats test (#2393)
Resolve IndexOutOfBoundsException, a regression from the first change in this issue.
* Call resolveDocCollection inside getCoreByCollection
* New BATS test to check URLs that the Solr Admin UI (or a user trying to navigate to such) might use
diff --git a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
index 20cec8f..6912af3 100644
--- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
+++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
@@ -40,7 +40,6 @@
import net.jcip.annotations.ThreadSafe;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.common.SolrException;
-import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.util.JsonSchemaValidator;
import org.apache.solr.common.util.PathTrie;
@@ -141,7 +140,12 @@
String collectionStr = queryParams.get(COLLECTION_PROP, origCorename);
collectionsList =
resolveCollectionListOrAlias(collectionStr); // &collection= takes precedence
- if (collectionsList.size() > 1) {
+ if (collectionsList.isEmpty()) {
+ if (!path.endsWith(CommonParams.INTROSPECT)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Resolved list of collections is empty");
+ }
+ } else if (collectionsList.size() > 1) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
"Request must be sent to a single collection "
@@ -150,29 +154,22 @@
+ collectionStr
+ "' resolves to "
+ this.collectionsList);
- }
- DocCollection collection = resolveDocCollection(collectionsList);
- if (collection == null) {
- if (!path.endsWith(CommonParams.INTROSPECT)) {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST, "no such collection or alias");
- }
} else {
+ String collectionName = collectionsList.get(0);
// Certain HTTP methods are only used for admin APIs, check for those and short-circuit
if (List.of("delete").contains(req.getMethod().toLowerCase(Locale.ROOT))) {
initAdminRequest(path);
return;
}
boolean isPreferLeader = (path.endsWith("/update") || path.contains("/update/"));
- core = getCoreByCollection(collection.getName(), isPreferLeader);
+ core = getCoreByCollection(collectionName, isPreferLeader);
if (core == null) {
// this collection exists , but this node does not have a replica for that collection
- extractRemotePath(collection.getName(), collection.getName());
+ extractRemotePath(collectionName, collectionName);
if (action == REMOTEQUERY) {
action = ADMIN_OR_REMOTEQUERY;
coreUrl = coreUrl.replace("/solr/", "/solr/____v2/c/");
- this.path =
- path = path.substring(prefix.length() + collection.getName().length() + 2);
+ this.path = path = path.substring(prefix.length() + collectionName.length() + 2);
return;
}
}
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index 2e5856f..72633c7 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -282,8 +282,6 @@
queryParams.get(COLLECTION_PROP, def)); // &collection= takes precedence
if (core == null) {
- // force update collection only if local clusterstate is outdated
- resolveDocCollection(collectionsList);
// lookup core from collection, or route away if need to
// route to 1st
String collectionName = collectionsList.isEmpty() ? null : collectionsList.get(0);
@@ -349,18 +347,21 @@
}
/**
- * Lookup the collection from the collection string (maybe comma delimited). Also sets {@link
- * #collectionsList} by side-effect. if {@code secondTry} is false then we'll potentially
- * recursively try this all one more time while ensuring the alias and collection info is sync'ed
- * from ZK.
+ * Resolves the specified collection name to a {@link DocCollection} object. If Solr is not in
+ * cloud mode, a {@link SolrException} is thrown. Returns null if the collection name is null or
+ * empty. Retrieves the {@link DocCollection} using the {@link ZkStateReader} from {@link
+ * CoreContainer}. If the collection is null, updates the state by refreshing aliases and forcing
+ * a collection update.
*/
- protected DocCollection resolveDocCollection(List<String> collectionsList) {
+ protected DocCollection resolveDocCollection(String collectionName) {
if (!cores.isZooKeeperAware()) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Solr not running in cloud mode ");
}
+ if (collectionName == null || collectionName.trim().isEmpty()) {
+ return null;
+ }
ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
- String collectionName = collectionsList.get(0);
Supplier<DocCollection> logic =
() -> zkStateReader.getClusterState().getCollectionOrNull(collectionName);
@@ -1064,15 +1065,21 @@
return result;
}
+ /**
+ * Retrieves a SolrCore instance associated with the specified collection name, with an option to
+ * prefer leader replicas. Makes a call to {@link #resolveDocCollection} which make an attempt to
+ * force update collection if it is not found in local cluster state
+ */
protected SolrCore getCoreByCollection(String collectionName, boolean isPreferLeader) {
ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
-
ClusterState clusterState = zkStateReader.getClusterState();
- DocCollection collection = clusterState.getCollectionOrNull(collectionName, true);
+ DocCollection collection = resolveDocCollection(collectionName);
+ // the usage of getCoreByCollection assumes that if null is returned, collection is found, but
+ // replicas might not
+ // have been created. Hence returning null here would be misleading...
if (collection == null) {
return null;
}
-
Set<String> liveNodes = clusterState.getLiveNodes();
if (isPreferLeader) {
diff --git a/solr/packaging/test/test_adminconsole_urls.bats b/solr/packaging/test/test_adminconsole_urls.bats
new file mode 100644
index 0000000..68fb79d
--- /dev/null
+++ b/solr/packaging/test/test_adminconsole_urls.bats
@@ -0,0 +1,38 @@
+#!/usr/bin/env bats
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load bats_helper
+
+setup() {
+ common_clean_setup
+}
+
+teardown() {
+ # save a snapshot of SOLR_HOME for failed tests
+ save_home_on_failure
+
+ solr stop -all >/dev/null 2>&1
+}
+
+@test "assert able to launch solr admin console" {
+ run solr start -c
+
+ run curl -s -o /dev/null -w "%{http_code}" http://localhost:${SOLR_PORT}/solr/
+
+ # Check if the HTTP response code is 200 (OK)
+ assert_output "200"
+}
