Skip to content

Commit

Permalink
SOLR-17153: Fix IndexOutOfBoundsException, add bats test (#2393)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aparnasuresh85 committed Apr 10, 2024
1 parent 453a23b commit 09e8d0c
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 24 deletions.
23 changes: 10 additions & 13 deletions solr/core/src/java/org/apache/solr/api/V2HttpCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -141,7 +140,12 @@ public void call(SolrQueryRequest req, SolrQueryResponse rsp) {
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 "
Expand All @@ -150,29 +154,22 @@ public void call(SolrQueryRequest req, SolrQueryResponse rsp) {
+ 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;
}
}
Expand Down
29 changes: 18 additions & 11 deletions solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,6 @@ protected void init() throws Exception {
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);
Expand Down Expand Up @@ -349,18 +347,21 @@ protected void init() throws Exception {
}

/**
* 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);

Expand Down Expand Up @@ -1064,15 +1065,21 @@ private Map<String, Integer> checkStateVersionsAreValid(String stateVer) {
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) {
Expand Down
38 changes: 38 additions & 0 deletions solr/packaging/test/test_adminconsole_urls.bats
Original file line number Diff line number Diff line change
@@ -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"
}

0 comments on commit 09e8d0c

Please sign in to comment.