SOLR-7988: LukeRequest on default path is broken with CloudSolrClient

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1700580 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -179,6 +179,8 @@ Bug Fixes
   of stats.field to prevent ArrayIndexOutOfBoundsException in a distributed search when a large precision
   is selected and a large number of values exist in each shard (hossman)
 
+* SOLR-7988: SolrJ could not make requests to handlers with '/admin/' prefix (noble , ludovic Boutros)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/cloud/ZkController.java

@@ -73,6 +73,7 @@ import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.cloud.ZooKeeperException;
 import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.URLUtil;
@@ -663,7 +664,7 @@ public final class ZkController {
       if (!zkRunOnly) {
         overseerElector = new LeaderElector(zkClient);
         this.overseer = new Overseer(shardHandler, updateShardHandler,
-            CoreContainer.CORES_HANDLER_PATH, zkStateReader, this, cloudConfig);
+            CommonParams.CORES_HANDLER_PATH, zkStateReader, this, cloudConfig);
         ElectionContext context = new OverseerElectionContext(zkClient,
             overseer, getNodeName());
         overseerElector.setup(context);

solr/core/src/java/org/apache/solr/cloud/rule/SnitchContext.java

@@ -21,16 +21,14 @@ import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.Callable;
-import java.util.concurrent.Future;
 
-import org.apache.http.client.methods.HttpGet;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.BinaryResponseParser;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.GenericSolrRequest;
 import org.apache.solr.client.solrj.response.SimpleSolrResponse;
+import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -89,7 +87,7 @@ public class SnitchContext implements RemoteCallback {
     //todo batch all requests to the same server
 
     try {
-      SimpleSolrResponse rsp = invoke(snitchInfo.getCoreContainer().getUpdateShardHandler(), url, CoreContainer.CORES_HANDLER_PATH, params);
+      SimpleSolrResponse rsp = invoke(snitchInfo.getCoreContainer().getUpdateShardHandler(), url, CommonParams.CORES_HANDLER_PATH, params);
       Map<String, Object> returnedVal = (Map<String, Object>) rsp.getResponse().get(klas);
       if(exception == null){
 //        log this

solr/core/src/java/org/apache/solr/core/CoreContainer.java

@@ -30,7 +30,6 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
@@ -69,6 +68,7 @@ import org.slf4j.LoggerFactory;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.Collections.EMPTY_MAP;
+import static org.apache.solr.common.params.CommonParams.*;
 import static org.apache.solr.security.AuthenticationPlugin.AUTHENTICATION_PLUGIN_PROP;
 
 
@@ -128,11 +128,6 @@ public class CoreContainer {
 
   private final JarRepository jarRepository = new JarRepository(this);
 
-  public static final String CORES_HANDLER_PATH = "/admin/cores";
-  public static final String COLLECTIONS_HANDLER_PATH = "/admin/collections";
-  public static final String INFO_HANDLER_PATH = "/admin/info";
-  public static final String CONFIGSETS_HANDLER_PATH = "/admin/configs";
-
   private PluginBag<SolrRequestHandler> containerHandlers = new PluginBag<>(SolrRequestHandler.class, null);
 
   private boolean asyncSolrCoreLoad;
@@ -412,8 +407,8 @@ public class CoreContainer {
     containerHandlers.put(CORES_HANDLER_PATH, coreAdminHandler);
     configSetsHandler = createHandler(cfg.getConfigSetsHandlerClass(), ConfigSetsHandler.class);
     containerHandlers.put(CONFIGSETS_HANDLER_PATH, configSetsHandler);
-    containerHandlers.put("/admin/authorization", securityConfHandler);
+    containerHandlers.put(AUTHZ_PATH, securityConfHandler);
-    containerHandlers.put("/admin/authentication", securityConfHandler);
+    containerHandlers.put(AUTHC_PATH, securityConfHandler);
     if(pkiAuthenticationPlugin != null)
       containerHandlers.put(PKIAuthenticationPlugin.PATH, pkiAuthenticationPlugin.getRequestHandler());
 

solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java

@@ -20,6 +20,7 @@ package org.apache.solr.servlet;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.commons.lang.StringUtils;
+import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.SolrCore;
 
@@ -64,7 +65,7 @@ public final class LoadAdminUiServlet extends BaseSolrServlet {
         };
         String[] replace = new String[] {
             StringEscapeUtils.escapeJavaScript(request.getContextPath()),
-            StringEscapeUtils.escapeJavaScript(CoreContainer.CORES_HANDLER_PATH),
+            StringEscapeUtils.escapeJavaScript(CommonParams.CORES_HANDLER_PATH),
             StringEscapeUtils.escapeJavaScript(pack.getSpecificationVersion())
         };
         

solr/core/src/test/org/apache/solr/cloud/rule/RulesTest.java

@@ -37,7 +37,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
-import static org.apache.solr.core.CoreContainer.COLLECTIONS_HANDLER_PATH;
+import static org.apache.solr.common.params.CommonParams.COLLECTIONS_HANDLER_PATH;
 
 public class RulesTest extends AbstractFullDistribZkTestBase {
   static final Logger log = LoggerFactory.getLogger(RulesTest.class);

solr/core/src/test/org/apache/solr/handler/TestSolrConfigHandlerCloud.java

@@ -25,12 +25,15 @@ import java.util.Map;
 import java.util.Objects;
 
 import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.request.LukeRequest;
 import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.RequestParams;
@@ -73,19 +76,45 @@ public class TestSolrConfigHandlerCloud extends AbstractFullDistribZkTestBase {
     setupHarnesses();
     testReqHandlerAPIs();
     testReqParams();
+    testAdminPath();
+  }
+
+  private void testAdminPath() throws Exception{
+    String testServerBaseUrl = getRandomServer(cloudClient,"collection1");
+    RestTestHarness writeHarness = restTestHarnesses.get(random().nextInt(restTestHarnesses.size()));
+    String payload = "{\n" +
+        "'create-requesthandler' : { 'name' : '/admin/luke', " +
+        "'class': 'org.apache.solr.handler.DumpRequestHandler'}}";
+
+    TestSolrConfigHandler.runConfigCommand(writeHarness, "/config?wt=json", payload);
+
+
+    TestSolrConfigHandler.testForResponseElement(writeHarness,
+        testServerBaseUrl,
+        "/config/overlay?wt=json",
+        cloudClient,
+        Arrays.asList("overlay", "requestHandler", "/admin/luke", "class"),
+        "org.apache.solr.handler.DumpRequestHandler",
+        10);
+
+   NamedList<Object> rsp = cloudClient.request(new LukeRequest());
+   System.out.println(rsp);
   }
 
   private void testReqHandlerAPIs() throws Exception {
-    DocCollection coll = cloudClient.getZkStateReader().getClusterState().getCollection("collection1");
+    String testServerBaseUrl = getRandomServer(cloudClient,"collection1");
+    RestTestHarness writeHarness = restTestHarnesses.get(random().nextInt(restTestHarnesses.size()));
+    TestSolrConfigHandler.reqhandlertests(writeHarness, testServerBaseUrl , cloudClient);
+  }
+
+  public static String getRandomServer(CloudSolrClient cloudClient, String collName) {
+    DocCollection coll = cloudClient.getZkStateReader().getClusterState().getCollection(collName);
     List<String> urls = new ArrayList<>();
     for (Slice slice : coll.getSlices()) {
       for (Replica replica : slice.getReplicas())
         urls.add(""+replica.get(ZkStateReader.BASE_URL_PROP) + "/"+replica.get(ZkStateReader.CORE_NAME_PROP));
     }
-
+    return urls.get(random().nextInt(urls.size()));
-    RestTestHarness writeHarness = restTestHarnesses.get(random().nextInt(restTestHarnesses.size()));
-    String testServerBaseUrl = urls.get(random().nextInt(urls.size()));
-    TestSolrConfigHandler.reqhandlertests(writeHarness, testServerBaseUrl , cloudClient);
   }
 
   private void testReqParams() throws Exception{

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

@@ -61,6 +61,7 @@ import java.net.ConnectException;
 import java.net.SocketException;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -74,11 +75,16 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
+import static org.apache.solr.common.params.CommonParams.AUTHC_PATH;
+import static org.apache.solr.common.params.CommonParams.AUTHZ_PATH;
+import static org.apache.solr.common.params.CommonParams.COLLECTIONS_HANDLER_PATH;
+import static org.apache.solr.common.params.CommonParams.CONFIGSETS_HANDLER_PATH;
+import static org.apache.solr.common.params.CommonParams.CORES_HANDLER_PATH;
+
 /**
  * SolrJ client class to communicate with SolrCloud.
  * Instances of this class communicate with Zookeeper to discover
@@ -799,6 +805,12 @@ public class CloudSolrClient extends SolrClient {
       collection = (reqParams != null) ? reqParams.get("collection", getDefaultCollection()) : getDefaultCollection();
     return requestWithRetryOnStaleState(request, 0, collection);
   }
+  private static final Set<String> ADMIN_PATHS = new HashSet<>(Arrays.asList(
+      CORES_HANDLER_PATH,
+      COLLECTIONS_HANDLER_PATH,
+      CONFIGSETS_HANDLER_PATH,
+      AUTHC_PATH,
+      AUTHZ_PATH));
 
   /**
    * As this class doesn't watch external collections on the client side,
@@ -816,7 +828,8 @@ public class CloudSolrClient extends SolrClient {
     // collections is stale and needs to be refreshed ... this code has no impact on internal collections
     String stateVerParam = null;
     List<DocCollection> requestedCollections = null;
-    if (collection != null && !request.getPath().startsWith("/admin")) { // don't do _stateVer_ checking for admin requests
+    boolean isAdmin = ADMIN_PATHS.contains(request.getPath());
+    if (collection != null &&  !isAdmin) { // don't do _stateVer_ checking for admin requests
       Set<String> requestedCollectionNames = getCollectionNames(getZkStateReader().getClusterState(), collection);
 
       StringBuilder stateVerParamBuilder = null;
@@ -871,7 +884,7 @@ public class CloudSolrClient extends SolrClient {
 
       Throwable rootCause = SolrException.getRootCause(exc);
       // don't do retry support for admin requests or if the request doesn't have a collection specified
-      if (collection == null || request.getPath().startsWith("/admin")) {
+      if (collection == null || isAdmin) {
         if (exc instanceof SolrServerException) {
           throw (SolrServerException)exc;
         } else if (exc instanceof IOException) {
@@ -979,7 +992,7 @@ public class CloudSolrClient extends SolrClient {
       reqParams = new ModifiableSolrParams();
     }
     List<String> theUrlList = new ArrayList<>();
-    if (request.getPath().startsWith("/admin/")) {
+    if (ADMIN_PATHS.contains(request.getPath())) {
       Set<String> liveNodes = clusterState.getLiveNodes();
       for (String liveNode : liveNodes) {
         theUrlList.add(zkStateReader.getBaseUrlForNodeName(liveNode));

solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java

@@ -163,6 +163,12 @@ public interface CommonParams {
 
   /** include header in the response */
   public static final String OMIT_HEADER = "omitHeader";
+  public static final String CORES_HANDLER_PATH = "/admin/cores";
+  public static final String COLLECTIONS_HANDLER_PATH = "/admin/collections";
+  public static final String INFO_HANDLER_PATH = "/admin/info";
+  public static final String CONFIGSETS_HANDLER_PATH = "/admin/configs";
+  public static final String AUTHZ_PATH = "/admin/authorization";
+  public static final String AUTHC_PATH = "/admin/authentication";
 
   /** valid values for: <code>echoParams</code> */
   public enum EchoParamStyle {