SOLR-17300: Copy existing listeners on re-creation of Http2SolrClient

solr/CHANGES.txt

@@ -116,6 +116,9 @@ Improvements
 * SOLR-17274: Allow JSON atomic updates to use multiple modifiers or a modifier like 'set' as a field name
   if child docs are not enabled. (Calvin Smith, David Smiley)
 
+* SOLR-17300: Http2SolrClient.Builder.withHttpClient now copies HttpListenerFactory (e.g. for auth, metrics, traces, etc.)
+  (Sanjay Dutt, David Smiley)
+
 Optimizations
 ---------------------
 * SOLR-17257: Both Minimize Cores and the Affinity replica placement strategies would over-gather

solr/core/src/java/org/apache/solr/handler/IndexFetcher.java

@@ -269,8 +269,6 @@ private SolrClient createSolrClient(
     Http2SolrClient httpClient =
         new Http2SolrClient.Builder(leaderBaseUrl)
             .withHttpClient(updateShardHandler.getRecoveryOnlyHttpClient())
-            .withListenerFactory(
-                updateShardHandler.getRecoveryOnlyHttpClient().getListenerFactory())
             .withBasicAuthCredentials(httpBasicAuthUser, httpBasicAuthPassword)
             .withIdleTimeout(soTimeout, TimeUnit.MILLISECONDS)
             .withConnectionTimeout(connTimeout, TimeUnit.MILLISECONDS)

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

@@ -0,0 +1,121 @@
+/*
+ * 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 static org.apache.solr.client.solrj.response.RequestStatusState.COMPLETED;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.CloudLegacySolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+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.client.solrj.response.RequestStatusState;
+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.util.SecurityJson;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class RecoveryZkTestWithAuth extends SolrCloudTestCase {
+  @BeforeClass
+  public static void setupCluster() throws Exception {
+    cluster =
+        configureCluster(1)
+            .addConfig("conf", configset("cloud-minimal"))
+            .withSecurityJson(SecurityJson.SIMPLE)
+            .configure();
+  }
+
+  private <T extends SolrRequest<? extends SolrResponse>> T withBasicAuth(T req) {
+    req.setBasicAuthCredentials(SecurityJson.USER, SecurityJson.PASS);
+    return req;
+  }
+
+  private QueryResponse queryWithBasicAuth(SolrClient client, SolrQuery q)
+      throws IOException, SolrServerException {
+    return withBasicAuth(new QueryRequest(q)).process(client);
+  }
+
+  @Test
+  public void testRecoveryWithAuthEnabled() throws Exception {
+    final String collection = "recoverytestwithauth";
+    withBasicAuth(CollectionAdminRequest.createCollection(collection, "conf", 1, 1))
+        .process(cluster.getSolrClient());
+    waitForState(
+        "Expected a collection with one shard and one replicas", collection, clusterShape(1, 1));
+    try (SolrClient solrClient =
+        cluster.basicSolrClientBuilder().withDefaultCollection(collection).build()) {
+      UpdateRequest commitReq = new UpdateRequest();
+      withBasicAuth(commitReq);
+      for (int i = 0; i < 500; i++) {
+        UpdateRequest req = new UpdateRequest();
+        withBasicAuth(req).add(sdoc("id", i, "name", "name = " + i));
+        req.process(solrClient, collection);
+        if (i % 10 == 0) {
+          commitReq.commit(solrClient, collection);
+        }
+      }
+      commitReq.commit(solrClient, collection);
+
+      withBasicAuth(CollectionAdminRequest.addReplicaToShard(collection, "shard1"));
+      CollectionAdminRequest.AddReplica addReplica =
+          CollectionAdminRequest.addReplicaToShard(collection, "shard1");
+      withBasicAuth(addReplica);
+      RequestStatusState status = addReplica.processAndWait(collection, solrClient, 120);
+      assertEquals(COMPLETED, status);
+      cluster
+          .getZkStateReader()
+          .waitForState(collection, 120, TimeUnit.SECONDS, clusterShape(1, 2));
+      DocCollection state = getCollectionState(collection);
+      assertShardConsistency(state.getSlice("shard1"), true);
+    }
+  }
+
+  private void assertShardConsistency(Slice shard, boolean expectDocs) throws Exception {
+    List<Replica> replicas = shard.getReplicas(r -> r.getState() == Replica.State.ACTIVE);
+    long[] numCounts = new long[replicas.size()];
+    int i = 0;
+    for (Replica replica : replicas) {
+      try (var client =
+          new HttpSolrClient.Builder(replica.getBaseUrl())
+              .withDefaultCollection(replica.getCoreName())
+              .withHttpClient(((CloudLegacySolrClient) cluster.getSolrClient()).getHttpClient())
+              .build()) {
+        var q = new SolrQuery("*:*");
+        q.add("distrib", "false");
+        numCounts[i] = queryWithBasicAuth(client, q).getResults().getNumFound();
+        i++;
+      }
+    }
+    for (int j = 1; j < replicas.size(); j++) {
+      if (numCounts[j] != numCounts[j - 1])
+        fail("Mismatch in counts between replicas"); // TODO improve this!
+      if (numCounts[j] == 0 && expectDocs)
+        fail("Expected docs on shard " + shard.getName() + " but found none");
+    }
+  }
+}

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

@@ -145,14 +145,11 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) {
     assert ObjectReleaseTracker.track(this);
   }
 
+  @Deprecated(since = "9.7")
   public void addListenerFactory(HttpListenerFactory factory) {
     this.listenerFactory.add(factory);
   }
 
-  public List<HttpListenerFactory> getListenerFactory() {
-    return listenerFactory;
-  }
-
   // internal usage only
   HttpClient getHttpClient() {
     return httpClient;
@@ -851,11 +848,6 @@ public static class Builder
 
     protected Long keyStoreReloadIntervalSecs;
 
-    public Http2SolrClient.Builder withListenerFactory(List<HttpListenerFactory> listenerFactory) {
-      this.listenerFactory = listenerFactory;
-      return this;
-    }
-
     private List<HttpListenerFactory> listenerFactory;
 
     public Builder() {
@@ -882,6 +874,11 @@ public Builder(String baseSolrUrl) {
       this.baseSolrUrl = baseSolrUrl;
     }
 
+    public Http2SolrClient.Builder withListenerFactory(List<HttpListenerFactory> listenerFactory) {
+      this.listenerFactory = listenerFactory;
+      return this;
+    }
+
     public HttpSolrClientBuilderBase<Http2SolrClient.Builder, Http2SolrClient> withSSLConfig(
         SSLConfig sslConfig) {
       this.sslConfig = sslConfig;
@@ -1054,6 +1051,10 @@ public Builder withHttpClient(Http2SolrClient http2SolrClient) {
       if (this.urlParamNames == null) {
         this.urlParamNames = http2SolrClient.urlParamNames;
       }
+      if (this.listenerFactory == null) {
+        this.listenerFactory = new ArrayList<HttpListenerFactory>();
+        http2SolrClient.listenerFactory.forEach(this.listenerFactory::add);
+      }
       return this;
     }