SOLR-17066: Add 'defaultCollection' to SolrClients (#2066)

This commit adds a setter ('withDefaultCollection(String)') to all
SolrClient builders, and propagates the field down to each client
implementation.  All SolrClient implementations now have a getter to
fetch this property ('getDefaultCollection').

This will help users replace the anti-pattern of baking the collection/
core name into their client's base URL to achieve a sort of "poor
man's" default.

solr/CHANGES.txt

@@ -124,6 +124,10 @@ Improvements
 
 * SOLR-17063: Do not retain log param references in LogWatcher (Michael Gibney)
 
+* SOLR-17066: SolrClient builders now allow users to specify a "default" collection or core
+  using the `withDefaultCollection` method.  This is preferable to including the collection
+  in the base URL accepted by certain client implementations. (Jason Gerlowski)
+
 Optimizations
 ---------------------
 * SOLR-17084: LBSolrClient (used by CloudSolrClient) now returns the count of core tracked as not live AKA zombies

solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java

@@ -52,6 +52,7 @@ public abstract class SolrClient implements Serializable, Closeable {
   private static final long serialVersionUID = 1L;
 
   private DocumentObjectBinder binder;
+  protected String defaultCollection;
 
   /**
    * Adds a collection of documents
@@ -1215,4 +1216,13 @@ public DocumentObjectBinder getBinder() {
   public SolrRequest.SolrClientContext getContext() {
     return SolrRequest.SolrClientContext.CLIENT;
   }
+
+  /**
+   * Gets the collection used by default for collection or core-based requests
+   *
+   * <p>If no value is specified at client-creation time, this method will return null.
+   */
+  public String getDefaultCollection() {
+    return defaultCollection;
+  }
 }

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

@@ -167,7 +167,6 @@ public static class Builder extends SolrClientBuilder<Builder> {
     protected boolean shardLeadersOnly = true;
     protected boolean directUpdatesToLeadersOnly = false;
     protected boolean parallelUpdates = true;
-    protected String defaultCollection;
     protected long retryExpiryTimeNano =
         TimeUnit.NANOSECONDS.convert(3, TimeUnit.SECONDS); // 3 seconds or 3 million nanos
     protected ClusterStateProvider stateProvider;
@@ -343,12 +342,6 @@ public Builder withParallelUpdates(boolean parallelUpdates) {
       return this;
     }
 
-    /** Sets the default collection for request. */
-    public Builder withDefaultCollection(String collection) {
-      this.defaultCollection = collection;
-      return this;
-    }
-
     /**
      * Sets the Zk connection timeout
      *

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

@@ -90,7 +90,6 @@ public abstract class CloudSolrClient extends SolrClient {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  protected volatile String defaultCollection;
   // no of times collection state to be reloaded if stale state error is received
   private static final int MAX_STALE_RETRIES =
       Integer.parseInt(System.getProperty("cloudSolrClientMaxStaleRetries", "5"));
@@ -292,11 +291,6 @@ public RequestWriter getRequestWriter() {
     return getLbClient().getRequestWriter();
   }
 
-  /** Gets the default collection for request */
-  public String getDefaultCollection() {
-    return defaultCollection;
-  }
-
   /** Gets whether direct updates are sent in parallel */
   public boolean isParallelUpdates() {
     return parallelUpdates;

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

@@ -149,6 +149,7 @@ protected ConcurrentUpdateHttp2SolrClient(Builder builder) {
     this.runners = new ArrayDeque<>();
     this.streamDeletes = builder.streamDeletes;
     this.basePath = builder.baseSolrUrl;
+    this.defaultCollection = builder.defaultCollection;
     this.pollQueueTimeMillis = builder.pollQueueTimeMillis;
     this.stallTimeMillis = Integer.getInteger("solr.cloud.client.stallTime", 15000);
 
@@ -359,6 +360,7 @@ private void addRunner() {
   @Override
   public NamedList<Object> request(final SolrRequest<?> request, String collection)
       throws SolrServerException, IOException {
+    if (collection == null) collection = defaultCollection;
     if (!(request instanceof UpdateRequest)) {
       request.setBasePath(basePath);
       return client.request(request, collection);
@@ -697,6 +699,7 @@ public void shutdownNow() {
   public static class Builder {
     protected Http2SolrClient client;
     protected String baseSolrUrl;
+    protected String defaultCollection;
     protected int queueSize = 10;
     protected int threadCount;
     protected ExecutorService executorService;
@@ -787,6 +790,12 @@ public Builder neverStreamDeletes() {
       return this;
     }
 
+    /** Sets a default collection for collection-based requests. */
+    public Builder withDefaultCollection(String defaultCollection) {
+      this.defaultCollection = defaultCollection;
+      return this;
+    }
+
     /**
      * @param pollQueueTime time for an open connection to wait for updates when the queue is empty.
      */

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

@@ -116,6 +116,7 @@ protected ConcurrentUpdateSolrClient(Builder builder) {
     this.soTimeout = builder.socketTimeoutMillis;
     this.pollQueueTimeMillis = builder.pollQueueTime;
     this.stallTimeMillis = Integer.getInteger("solr.cloud.client.stallTime", 15000);
+    this.defaultCollection = builder.defaultCollection;
 
     // make sure the stall time is larger than the polling time
     // to give a chance for the queue to change
@@ -475,6 +476,7 @@ public void setCollection(String collection) {
   @Override
   public NamedList<Object> request(final SolrRequest<?> request, String collection)
       throws SolrServerException, IOException {
+    if (collection == null) collection = defaultCollection;
     if (!(request instanceof UpdateRequest)) {
       return client.request(request, collection);
     }

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

@@ -191,6 +191,7 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) {
       this.parser = builder.responseParser;
     }
     updateDefaultMimeTypeForParser();
+    this.defaultCollection = builder.defaultCollection;
     if (builder.requestTimeoutMillis != null) {
       this.requestTimeoutMillis = builder.requestTimeoutMillis;
     } else {
@@ -554,7 +555,7 @@ public void onFailure(Response response, Throwable failure) {
   @Override
   public NamedList<Object> request(SolrRequest<?> solrRequest, String collection)
       throws SolrServerException, IOException {
-
+    if (collection == null) collection = defaultCollection;
     String url = getRequestPath(solrRequest, collection);
     Throwable abortCause = null;
     Request req = null;
@@ -1070,6 +1071,7 @@ public static class Builder {
     private ExecutorService executor;
     protected RequestWriter requestWriter;
     protected ResponseParser responseParser;
+    protected String defaultCollection;
     private Set<String> urlParamNames;
     private CookieStore cookieStore = getDefaultCookieStore();
     private String proxyHost;
@@ -1194,6 +1196,12 @@ public Builder withResponseParser(ResponseParser responseParser) {
       return this;
     }
 
+    /** Sets a default collection for collection-based requests. */
+    public Builder withDefaultCollection(String defaultCollection) {
+      this.defaultCollection = defaultCollection;
+      return this;
+    }
+
     public Builder withFollowRedirects(boolean followRedirects) {
       this.followRedirects = followRedirects;
       return this;

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

@@ -188,6 +188,7 @@ protected HttpSolrClient(Builder builder) {
     this.soTimeout = builder.socketTimeoutMillis;
     this.useMultiPartPost = builder.useMultiPartPost;
     this.urlParamNames = builder.urlParamNames;
+    this.defaultCollection = builder.defaultCollection;
   }
 
   public Set<String> getUrlParamNames() {
@@ -242,6 +243,7 @@ public NamedList<Object> request(final SolrRequest<?> request, final ResponsePar
   public NamedList<Object> request(
       final SolrRequest<?> request, final ResponseParser processor, String collection)
       throws SolrServerException, IOException {
+    if (collection == null) collection = defaultCollection;
     HttpRequestBase method = createMethod(request, collection);
     setBasicAuthHeader(request, method);
     if (request.getHeaders() != null) {

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

@@ -23,7 +23,6 @@
 import java.net.SocketException;
 import java.net.SocketTimeoutException;
 import java.util.Arrays;
-import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -86,9 +85,11 @@
 public class LBHttp2SolrClient extends LBSolrClient {
   private final Http2SolrClient solrClient;
 
-  private LBHttp2SolrClient(Http2SolrClient solrClient, List<String> baseSolrUrls) {
-    super(baseSolrUrls);
-    this.solrClient = solrClient;
+  private LBHttp2SolrClient(Builder builder) {
+    super(Arrays.asList(builder.baseSolrUrls));
+    this.solrClient = builder.http2SolrClient;
+    this.aliveCheckIntervalMillis = builder.aliveCheckIntervalMillis;
+    this.defaultCollection = builder.defaultCollection;
   }
 
   @Override
@@ -258,6 +259,7 @@ public static class Builder {
     private final String[] baseSolrUrls;
     private long aliveCheckIntervalMillis =
         TimeUnit.MILLISECONDS.convert(60, TimeUnit.SECONDS); // 1 minute between checks
+    protected String defaultCollection;
 
     public Builder(Http2SolrClient http2Client, String... baseSolrUrls) {
       this.http2SolrClient = http2Client;
@@ -279,11 +281,14 @@ public LBHttp2SolrClient.Builder setAliveCheckInterval(int aliveCheckInterval, T
       return this;
     }
 
+    /** Sets a default collection for collection-based requests. */
+    public LBHttp2SolrClient.Builder withDefaultCollection(String defaultCollection) {
+      this.defaultCollection = defaultCollection;
+      return this;
+    }
+
     public LBHttp2SolrClient build() {
-      LBHttp2SolrClient solrClient =
-          new LBHttp2SolrClient(this.http2SolrClient, Arrays.asList(this.baseSolrUrls));
-      solrClient.aliveCheckIntervalMillis = this.aliveCheckIntervalMillis;
-      return solrClient;
+      return new LBHttp2SolrClient(this);
     }
   }
 }

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

@@ -93,6 +93,10 @@ protected LBHttpSolrClient(Builder builder) {
         builder.httpClient == null
             ? constructClient(builder.baseSolrUrls.toArray(new String[0]))
             : builder.httpClient;
+    this.defaultCollection = builder.defaultCollection;
+    if (httpSolrClientBuilder != null && this.defaultCollection != null) {
+      httpSolrClientBuilder.defaultCollection = this.defaultCollection;
+    }
     this.connectionTimeoutMillis = builder.connectionTimeoutMillis;
     this.soTimeoutMillis = builder.socketTimeoutMillis;
     this.parser = builder.responseParser;

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

@@ -574,6 +574,7 @@ public NamedList<Object> request(
     final int maxTries = (numServersToTry == null ? serverList.length : numServersToTry.intValue());
     int numServersTried = 0;
     Map<String, ServerWrapper> justFailed = null;
+    if (collection == null) collection = defaultCollection;
 
     boolean timeAllowedExceeded = false;
     long timeAllowedNano = getTimeAllowedInNanos(request);

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

@@ -41,6 +41,7 @@ public abstract class SolrClientBuilder<B extends SolrClientBuilder<B>> {
   protected int socketTimeoutMillis = 120000; // 120 seconds
   private boolean socketTimeoutMillisUpdate = false;
   protected boolean followRedirects = false;
+  protected String defaultCollection;
   protected Set<String> urlParamNames;
 
   /** The solution for the unchecked cast warning. */
@@ -97,6 +98,11 @@ public B withFollowRedirects(boolean followRedirects) {
     return getThis();
   }
 
+  public B withDefaultCollection(String defaultCollection) {
+    this.defaultCollection = defaultCollection;
+    return getThis();
+  }
+
   /**
    * Tells {@link Builder} that created clients should obey the following timeout when connecting to
    * Solr servers.

solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientBuilderTest.java

@@ -161,4 +161,15 @@ public void testProvideExternalClient() throws IOException {
     // it's external, should be NOT closed when closing CloudSolrClient
     verify(http2Client, never()).close();
   }
+
+  @Test
+  public void testDefaultCollectionPassedFromBuilderToClient() throws IOException {
+    try (CloudHttp2SolrClient createdClient =
+        new CloudHttp2SolrClient.Builder(
+                Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
+            .withDefaultCollection("aCollection")
+            .build()) {
+      assertEquals("aCollection", createdClient.getDefaultCollection());
+    }
+  }
 }

solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBuilderTest.java

@@ -101,4 +101,14 @@ public void test0Timeouts() throws IOException {
       assertNotNull(createdClient);
     }
   }
+
+  @Test
+  public void testDefaultCollectionPassedFromBuilderToClient() throws IOException {
+    try (CloudSolrClient createdClient =
+        new Builder(Collections.singletonList(ANY_ZK_HOST), Optional.of(ANY_CHROOT))
+            .withDefaultCollection("aCollection")
+            .build()) {
+      assertEquals("aCollection", createdClient.getDefaultCollection());
+    }
+  }
 }

solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientBuilderTest.java

@@ -23,6 +23,7 @@
 import java.net.SocketTimeoutException;
 import java.util.concurrent.TimeUnit;
 import org.apache.solr.SolrTestCase;
+import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.ConcurrentUpdateSolrClient.Builder;
 import org.junit.Test;
@@ -71,4 +72,14 @@ public void testSocketTimeoutOnCommit() throws IOException, SolrServerException
       // else test passses
     }
   }
+
+  @Test
+  public void testDefaultCollectionPassedFromBuilderToClient() throws IOException {
+    try (SolrClient createdClient =
+        new ConcurrentUpdateSolrClient.Builder("someurl")
+            .withDefaultCollection("aCollection")
+            .build()) {
+      assertEquals("aCollection", createdClient.getDefaultCollection());
+    }
+  }
 }

solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderTest.java

@@ -22,6 +22,7 @@
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.solr.SolrTestCase;
 import org.apache.solr.client.solrj.ResponseParser;
+import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.junit.Test;
@@ -83,4 +84,12 @@ public void testDefaultsToBinaryResponseParserWhenNoneProvided() throws IOExcept
       assertTrue(usedParser instanceof BinaryResponseParser);
     }
   }
+
+  @Test
+  public void testDefaultCollectionPassedFromBuilderToClient() throws IOException {
+    try (final SolrClient createdClient =
+        new Builder(ANY_BASE_SOLR_URL).withDefaultCollection("aCollection").build()) {
+      assertEquals("aCollection", createdClient.getDefaultCollection());
+    }
+  }
 }

solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientBuilderTest.java

@@ -77,4 +77,15 @@ public void testUsesTimeoutProvidedByHttpClient() throws IOException {
     }
     HttpClientUtil.close(httpClient);
   }
+
+  @Test
+  public void testDefaultCollectionPassedFromBuilderToClient() throws IOException {
+    try (LBHttpSolrClient createdClient =
+        new LBHttpSolrClient.Builder()
+            .withBaseSolrUrl(ANY_BASE_SOLR_URL)
+            .withDefaultCollection("aCollection")
+            .build()) {
+      assertEquals("aCollection", createdClient.getDefaultCollection());
+    }
+  }
 }