SOLR-12849: keep a POST body's collection parameter when the path names multiple collections

changelog/unreleased/SOLR-12849.yml

@@ -0,0 +1,7 @@
+title: Keep a POST body's collection parameter when the path names multiple collections
+type: fixed
+authors:
+  - name: Nick Shanin
+links:
+  - name: SOLR-12849
+    url: https://issues.apache.org/jira/browse/SOLR-12849

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

@@ -780,12 +780,21 @@ protected void handleAdmin(SolrQueryResponse solrResp) {
    *
    * @see #getCollectionsList()
    */
-  protected void addCollectionParamIfNeeded(List<String> collections) {
-    if (collections.isEmpty()) {
+  protected void addCollectionParamIfNeeded(List<String> requestCollections) {
+    if (requestCollections.isEmpty()) {
       return;
     }
     assert cores.isZooKeeperAware();
-    String collectionParam = queryParams.get(COLLECTION_PROP);
+    // The collections list is computed from the URL params only. A collection param in a POST form
+    // body is not part of it, so resolve the aliases in that param instead of replacing it.
+    String collectionParam = getQueryParams().get(COLLECTION_PROP);
+    List<String> collections = requestCollections;
+    if (collectionParam != null && queryParams.get(COLLECTION_PROP) == null) {
+      collections = resolveCollectionListOrAlias(collectionParam);
+      if (collections.isEmpty()) {
+        return;
+      }
+    }
     // if there is no existing collection param and the core we go to is for the expected
     // collection, then we needn't add a collection param
     if (collectionParam == null

solr/core/src/test/org/apache/solr/cloud/AliasPostBodyTest.java

@@ -0,0 +1,96 @@
+/*
+ * 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.
+ */
+package org.apache.solr.cloud;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Tests that a "collection" parameter in the form body of a POST has its aliases resolved, and is
+ * otherwise left as it is.
+ */
+public class AliasPostBodyTest extends SolrCloudTestCase {
+
+  private static final String EMPTY_COLLECTION = "emptycoll";
+  private static final String DOC_COLLECTION = "doccoll";
+  private static final String EMPTY_ALIAS = "emptyalias";
+  private static final String DOC_ALIAS = "docalias";
+  private static final String BOTH_ALIAS = "bothalias";
+
+  @BeforeClass
+  public static void setupCluster() throws Exception {
+    configureCluster(1).addConfig("conf", configset("cloud-minimal")).configure();
+
+    for (String collection : new String[] {EMPTY_COLLECTION, DOC_COLLECTION}) {
+      CollectionAdminRequest.createCollection(collection, "conf", 1, 1)
+          .process(cluster.getSolrClient());
+      cluster.waitForActiveCollection(collection, 1, 1);
+    }
+    CollectionAdminRequest.createAlias(EMPTY_ALIAS, EMPTY_COLLECTION)
+        .process(cluster.getSolrClient());
+    CollectionAdminRequest.createAlias(DOC_ALIAS, DOC_COLLECTION).process(cluster.getSolrClient());
+    CollectionAdminRequest.createAlias(BOTH_ALIAS, EMPTY_COLLECTION + "," + DOC_COLLECTION)
+        .process(cluster.getSolrClient());
+    new UpdateRequest().add("id", "1").commit(cluster.getSolrClient(), DOC_COLLECTION);
+  }
+
+  /** Sends a form body POST to the path alias, with the collection parameter in the body. */
+  private static long postWithCollectionParam(String pathAlias, String collectionParam)
+      throws Exception {
+    ModifiableSolrParams params = new ModifiableSolrParams();
+    params.set("q", "*:*");
+    params.set("rows", "0");
+    params.set("collection", collectionParam);
+    SolrClient nodeClient = cluster.getJettySolrRunner(0).getSolrClient();
+    QueryResponse response =
+        new QueryRequest(params, SolrRequest.METHOD.POST).process(nodeClient, pathAlias);
+    return response.getResults().getNumFound();
+  }
+
+  @Test
+  public void testAliasInPostBodyIsResolved() throws Exception {
+    assertEquals(0, postWithCollectionParam(EMPTY_ALIAS, EMPTY_ALIAS));
+  }
+
+  @Test
+  public void testAliasInPostBodyDiffersFromThePathAlias() throws Exception {
+    assertEquals(1, postWithCollectionParam(EMPTY_ALIAS, DOC_ALIAS));
+  }
+
+  @Test
+  public void testCollectionInPostBodyIsNotReplacedByThePathCollection() throws Exception {
+    assertEquals(1, postWithCollectionParam(EMPTY_ALIAS, DOC_COLLECTION));
+  }
+
+  @Test
+  public void testBodyCollectionSurvivesTwoCollectionPathAlias() throws Exception {
+    // The path alias resolves to two collections, but the collection named by the
+    // request must still win. A body-only collection value is replaced with the joined
+    // path list inside addCollectionParamIfNeeded on the base code (see
+    // HttpSolrCallCollectionParamTest); this end-to-end case passes on base as well,
+    // because the SolrJ client also places the collection parameter in the URL, where
+    // it takes precedence over the path.
+    assertEquals(0, postWithCollectionParam(BOTH_ALIAS, EMPTY_COLLECTION));
+  }
+}

solr/core/src/test/org/apache/solr/servlet/HttpSolrCallCollectionParamTest.java

@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.solr.servlet;
+
+import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.util.List;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.common.cloud.Aliases;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.request.SolrQueryRequestBase;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Method-level test for {@link HttpSolrCall#addCollectionParamIfNeeded(List)}: a {@code collection}
+ * parameter supplied in a POST form body must not be replaced by the list of collections derived
+ * from the request path when that list holds more than one collection.
+ */
+public class HttpSolrCallCollectionParamTest extends SolrTestCase {
+
+  @BeforeClass
+  public static void setUpOnce() {
+    assumeWorkingMockito();
+  }
+
+  @Test
+  public void testAddCollectionParamIfNeededKeepsBodyValueForTwoCollectionPath() {
+    CoreContainer cores = mock(CoreContainer.class);
+    when(cores.isZooKeeperAware()).thenReturn(true);
+    when(cores.getAliases()).thenReturn(Aliases.EMPTY);
+
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getServletPath()).thenReturn("/bothalias");
+    when(request.getPathInfo()).thenReturn("/select");
+    HttpServletResponse response = mock(HttpServletResponse.class);
+
+    HttpSolrCall call = new HttpSolrCall(cores, request, response, false);
+    // The URL query string carries no collection parameter...
+    call.queryParams = SolrRequestParsers.parseQueryString("q=*:*");
+    // ...but the parsed request, which merges in the POST form body, carries one.
+    ModifiableSolrParams requestParams = new ModifiableSolrParams();
+    requestParams.set(COLLECTION_PROP, "bodycoll");
+    requestParams.set("q", "*:*");
+    call.solrReq = new SolrQueryRequestBase(null, requestParams);
+
+    // The request path names an alias that resolves to two collections.
+    call.addCollectionParamIfNeeded(List.of("emptycollection", "doccollection"));
+
+    assertEquals("bodycoll", call.solrReq.getParams().get(COLLECTION_PROP));
+  }
+}