This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 9cabc8e188a SOLR-10452: setQueryParams should be deprecated in favor
of SolrClientBuilder methods (#1253)
9cabc8e188a is described below
commit 9cabc8e188a0c9beba2047c21b3ab9299365586d
Author: Eric Pugh <[email protected]>
AuthorDate: Sat Dec 31 08:35:04 2022 -0500
SOLR-10452: setQueryParams should be deprecated in favor of
SolrClientBuilder methods (#1253)
Moved setQueryParams to the Builder, and renamed it to setUrlParamNames, to
make it clearer that it's just the name of url parameters.
Co-authored-by: Alex Deparvu <[email protected]>
Co-authored-by: David Smiley <[email protected]>
---
solr/CHANGES.txt | 4 ++
.../org/apache/solr/update/UpdateShardHandler.java | 10 ++--
.../solrj/impl/ConcurrentUpdateSolrClient.java | 14 +++++-
.../solrj/impl/DelegationTokenHttpSolrClient.java | 18 ++++----
.../solr/client/solrj/impl/Http2SolrClient.java | 40 +++++++++++++---
.../solr/client/solrj/impl/HttpSolrClient.java | 40 ++++++++++++----
.../solr/client/solrj/impl/LBHttp2SolrClient.java | 52 +++++++++++++--------
.../solr/client/solrj/impl/LBHttpSolrClient.java | 31 +++++++++++--
.../solr/client/solrj/impl/LBSolrClient.java | 19 --------
.../solr/client/solrj/impl/SolrClientBuilder.java | 6 +--
.../client/solrj/impl/BasicHttpSolrClientTest.java | 28 +++++-------
.../client/solrj/impl/Http2SolrClientTest.java | 53 ++++++++++++----------
.../client/solrj/impl/LBHttp2SolrClientTest.java | 53 ++++++++++++----------
13 files changed, 224 insertions(+), 144 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index c7366cda64e..5cb23c19109 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -74,6 +74,10 @@ Improvements
* SOLR-10462: Introduce Builder setter for pollQueueTime on
ConcurrentUpdateHttp2SolrClient. Deprecated
direct setter setPollQueueTime on ConcurrentUpdateHttp2SolrClient. (Eric
Pugh)
+
+* SOLR-10452: Introduce Builder setter withTheseParamNamesInTheUrl for
queryParams, renaming them to urlParamNames
+ to clarify they are parameter names, not the values. Deprecated direct
setter setQueryParams and addQueryParams
+ on SolrClients. (Eric Pugh, David Smiley, Alex Deparvu)
Optimizations
---------------------
diff --git a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java
b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java
index 38b17cc442f..4ce3d9d9e71 100644
--- a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java
+++ b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java
@@ -20,7 +20,6 @@ import static
org.apache.solr.util.stats.InstrumentedHttpRequestExecutor.KNOWN_M
import com.google.common.annotations.VisibleForTesting;
import java.lang.invoke.MethodHandles;
-import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
@@ -138,6 +137,10 @@ public class UpdateShardHandler implements SolrInfoBean {
HttpClientUtil.createClient(
clientParams, defaultConnectionManager, false,
httpRequestExecutor);
+ Set<String> urlParamNames =
+ Set.of(
+ DistributedUpdateProcessor.DISTRIB_FROM,
+ DistributingUpdateProcessorFactory.DISTRIB_UPDATE_PARAM);
Http2SolrClient.Builder updateOnlyClientBuilder = new
Http2SolrClient.Builder();
if (cfg != null) {
updateOnlyClientBuilder
@@ -145,12 +148,9 @@ public class UpdateShardHandler implements SolrInfoBean {
.idleTimeout(cfg.getDistributedSocketTimeout())
.maxConnectionsPerHost(cfg.getMaxUpdateConnectionsPerHost());
}
+ updateOnlyClientBuilder.withTheseParamNamesInTheUrl(urlParamNames);
updateOnlyClient = updateOnlyClientBuilder.build();
updateOnlyClient.addListenerFactory(updateHttpListenerFactory);
- Set<String> queryParams = new HashSet<>(2);
- queryParams.add(DistributedUpdateProcessor.DISTRIB_FROM);
- queryParams.add(DistributingUpdateProcessorFactory.DISTRIB_UPDATE_PARAM);
- updateOnlyClient.setQueryParams(queryParams);
ThreadFactory recoveryThreadFactory = new
SolrNamedThreadFactory("recoveryExecutor");
if (cfg != null && cfg.getMaxRecoveryThreads() > 0) {
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
index 5150318b339..7680b6e7f62 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
@@ -108,6 +108,7 @@ public class ConcurrentUpdateSolrClient extends SolrClient {
.withConnectionTimeout(builder.connectionTimeoutMillis)
.withSocketTimeout(builder.socketTimeoutMillis)
.withFollowRedirects(false)
+ .withTheseParamNamesInTheUrl(builder.urlParamNames)
.build();
this.queue = new LinkedBlockingQueue<>(builder.queueSize);
this.threadCount = builder.threadCount;
@@ -140,15 +141,26 @@ public class ConcurrentUpdateSolrClient extends
SolrClient {
}
}
+ /**
+ * @deprecated use {@link #getUrlParamNames()}
+ */
+ @Deprecated
public Set<String> getQueryParams() {
- return this.client.getQueryParams();
+ return getUrlParamNames();
+ }
+
+ public Set<String> getUrlParamNames() {
+ return this.client.getUrlParamNames();
}
/**
* Expert Method.
*
* @param queryParams set of param keys to only send via the query string
+ * @deprecated use {@link
ConcurrentUpdateSolrClient.Builder#withTheseParamNamesInTheUrl(Set)}
+ * instead
*/
+ @Deprecated
public void setQueryParams(Set<String> queryParams) {
this.client.setQueryParams(queryParams);
}
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
index 72132c597d1..02f5ba2ece8 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/DelegationTokenHttpSolrClient.java
@@ -17,11 +17,9 @@
package org.apache.solr.client.solrj.impl;
import java.io.IOException;
-import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
-import java.util.TreeSet;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
@@ -37,7 +35,6 @@ public class DelegationTokenHttpSolrClient extends
HttpSolrClient {
*/
protected DelegationTokenHttpSolrClient(Builder builder) {
super(builder);
- setQueryParams(new TreeSet<>(Arrays.asList(DELEGATION_TOKEN_PARAM)));
}
@Override
@@ -50,14 +47,15 @@ public class DelegationTokenHttpSolrClient extends
HttpSolrClient {
return super.createMethod(request, collection);
}
+ @Deprecated
@Override
- public void setQueryParams(Set<String> queryParams) {
- queryParams = queryParams == null ? Set.of(DELEGATION_TOKEN_PARAM) :
queryParams;
- if (!queryParams.contains(DELEGATION_TOKEN_PARAM)) {
- queryParams = new HashSet<>(queryParams);
- queryParams.add(DELEGATION_TOKEN_PARAM);
- queryParams = Collections.unmodifiableSet(queryParams);
+ public void setQueryParams(Set<String> urlParamNames) {
+ urlParamNames = urlParamNames == null ? Set.of(DELEGATION_TOKEN_PARAM) :
urlParamNames;
+ if (!urlParamNames.contains(DELEGATION_TOKEN_PARAM)) {
+ urlParamNames = new HashSet<>(urlParamNames);
+ urlParamNames.add(DELEGATION_TOKEN_PARAM);
+ urlParamNames = Collections.unmodifiableSet(urlParamNames);
}
- super.setQueryParams(queryParams);
+ super.setQueryParams(urlParamNames);
}
}
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index d5f821b7ba3..71553756a5b 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -128,7 +128,7 @@ public class Http2SolrClient extends SolrClient {
private static final List<String> errPath = Arrays.asList("metadata",
"error-class");
private HttpClient httpClient;
- private volatile Set<String> queryParams = Collections.emptySet();
+ private volatile Set<String> urlParamNames = Set.of();
private int idleTimeout;
private int requestTimeout;
@@ -186,6 +186,7 @@ public class Http2SolrClient extends SolrClient {
requestTimeout = builder.requestTimeout;
}
httpClient.setFollowRedirects(builder.followRedirects);
+ this.urlParamNames = builder.urlParamNames;
assert ObjectReleaseTracker.track(this);
}
@@ -679,7 +680,7 @@ public class Http2SolrClient extends SolrClient {
contentWriter.getContentType(), ByteBuffer.wrap(baos.getbuf(),
0, baos.size())));
} else if (streams == null || isMultipart) {
// send server list and request list as query string params
- ModifiableSolrParams queryParams =
calculateQueryParams(this.queryParams, wparams);
+ ModifiableSolrParams queryParams =
calculateQueryParams(this.urlParamNames, wparams);
queryParams.add(calculateQueryParams(solrRequest.getQueryParams(),
wparams));
Request req = httpClient.newRequest(url +
queryParams.toQueryString()).method(method);
return fillContentStream(req, streams, wparams, isMultipart);
@@ -991,6 +992,7 @@ public class Http2SolrClient extends SolrClient {
private ExecutorService executor;
protected RequestWriter requestWriter;
protected ResponseParser responseParser;
+ private Set<String> urlParamNames = Set.of();
public Builder() {}
@@ -1079,6 +1081,19 @@ public class Http2SolrClient extends SolrClient {
return this;
}
+ /**
+ * Expert Method
+ *
+ * @param urlParamNames set of param keys that are only sent via the query
string. Note that the
+ * param will be sent as a query string if the key is part of this Set
or the SolrRequest's
+ * query params.
+ * @see org.apache.solr.client.solrj.SolrRequest#getQueryParams
+ */
+ public Builder withTheseParamNamesInTheUrl(Set<String> urlParamNames) {
+ this.urlParamNames = urlParamNames;
+ return this;
+ }
+
/**
* Set maxConnectionsPerHost for http1 connections, maximum number http2
connections is limited
* by 4
@@ -1115,19 +1130,30 @@ public class Http2SolrClient extends SolrClient {
}
}
+ /**
+ * @deprecated use {@link #getUrlParamNames()}
+ */
+ @Deprecated
public Set<String> getQueryParams() {
- return queryParams;
+ return getUrlParamNames();
+ }
+
+ public Set<String> getUrlParamNames() {
+ return urlParamNames;
}
/**
* Expert Method
*
- * @param queryParams set of param keys to only send via the query string
Note that the param will
- * be sent as a query string if the key is part of this Set or the
SolrRequest's query params.
+ * @param urlParamNames set of param keys that are only sent via the query
string. Note that the
+ * param will be sent as a query string if the key is part of this Set
or the SolrRequest's
+ * query params.
* @see org.apache.solr.client.solrj.SolrRequest#getQueryParams
+ * @deprecated use {@link
Http2SolrClient.Builder#withTheseParamNamesInTheUrl(Set)} instead
*/
- public void setQueryParams(Set<String> queryParams) {
- this.queryParams = queryParams;
+ @Deprecated
+ public void setUrlParamNames(Set<String> urlParamNames) {
+ this.urlParamNames = urlParamNames;
}
private ModifiableSolrParams calculateQueryParams(
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
index 79d01a5c4f4..15015b4fcce 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
@@ -36,6 +36,7 @@ import java.util.Arrays;
import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
@@ -148,7 +149,7 @@ public class HttpSolrClient extends BaseHttpSolrClient {
private volatile boolean useMultiPartPost;
private final boolean internalClient;
- private volatile Set<String> queryParams = Collections.emptySet();
+ private volatile Set<String> urlParamNames = Set.of();
private volatile Integer connectionTimeout;
private volatile Integer soTimeout;
@@ -187,26 +188,36 @@ public class HttpSolrClient extends BaseHttpSolrClient {
this.connectionTimeout = builder.connectionTimeoutMillis;
this.soTimeout = builder.socketTimeoutMillis;
this.useMultiPartPost = builder.useMultiPartPost;
- this.queryParams = builder.queryParams;
+ this.urlParamNames = builder.urlParamNames;
}
+ /**
+ * @deprecated use {@link #getUrlParamNames()}
+ */
+ @Deprecated
public Set<String> getQueryParams() {
- return queryParams;
+ return getUrlParamNames();
+ }
+
+ public Set<String> getUrlParamNames() {
+ return urlParamNames;
}
/**
* Expert Method
*
- * @param queryParams set of param keys to only send via the query string
Note that the param will
- * be sent as a query string if the key is part of this Set or the
SolrRequest's query params.
+ * @param urlParamNames set of param keys to only send via the query string
Note that the param
+ * will be sent as a query string if the key is part of this Set or the
SolrRequest's query
+ * params.
* <p>{@link SolrClient} setters can be unsafe when the involved {@link
SolrClient} is used in
- * multiple threads simultaneously. To avoid this, use {@link
Builder#withQueryParams(Set)}.
+ * multiple threads simultaneously. To avoid this, use {@link
+ * Builder#withTheseParamNamesInTheUrl(Set)}.
* @see org.apache.solr.client.solrj.SolrRequest#getQueryParams
- * @deprecated use {@link Builder#withQueryParams(Set)} instead
+ * @deprecated use {@link Builder#withTheseParamNamesInTheUrl(Set)} instead
*/
@Deprecated
- public void setQueryParams(Set<String> queryParams) {
- this.queryParams = queryParams;
+ public void setQueryParams(Set<String> urlParamNames) {
+ this.urlParamNames = urlParamNames;
}
/**
@@ -435,7 +446,7 @@ public class HttpSolrClient extends BaseHttpSolrClient {
} else if (streams == null || isMultipart) {
// send server list and request list as query string params
- ModifiableSolrParams queryParams =
calculateQueryParams(this.queryParams, wparams);
+ ModifiableSolrParams queryParams =
calculateQueryParams(this.urlParamNames, wparams);
queryParams.add(calculateQueryParams(request.getQueryParams(),
wparams));
String fullQueryUrl = url + queryParams.toQueryString();
HttpEntityEnclosingRequestBase postOrPut =
@@ -1041,6 +1052,15 @@ public class HttpSolrClient extends BaseHttpSolrClient {
if
(this.invariantParams.get(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM)
== null) {
return new HttpSolrClient(this);
} else {
+ urlParamNames =
+ urlParamNames == null
+ ? Set.of(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM)
+ : urlParamNames;
+ if
(!urlParamNames.contains(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM))
{
+ urlParamNames = new HashSet<>(urlParamNames);
+
urlParamNames.add(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM);
+ }
+ urlParamNames = Set.copyOf(urlParamNames);
return new DelegationTokenHttpSolrClient(this);
}
}
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
index 04b1800ab94..a98ed89d73b 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java
@@ -23,6 +23,7 @@ import java.net.ConnectException;
import java.net.SocketException;
import java.net.SocketTimeoutException;
import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -82,25 +83,25 @@ import org.slf4j.MDC;
* @since solr 8.0
*/
public class LBHttp2SolrClient extends LBSolrClient {
- private final Http2SolrClient httpClient;
+ private final Http2SolrClient solrClient;
/**
* @deprecated Use {@link LBHttp2SolrClient.Builder} instead
*/
@Deprecated
- public LBHttp2SolrClient(Http2SolrClient httpClient, String... baseSolrUrls)
{
+ public LBHttp2SolrClient(Http2SolrClient solrClient, String... baseSolrUrls)
{
super(Arrays.asList(baseSolrUrls));
- this.httpClient = httpClient;
+ this.solrClient = solrClient;
}
- private LBHttp2SolrClient(Http2SolrClient httpClient, List<String>
baseSolrUrls) {
+ private LBHttp2SolrClient(Http2SolrClient solrClient, List<String>
baseSolrUrls) {
super(baseSolrUrls);
- this.httpClient = httpClient;
+ this.solrClient = solrClient;
}
@Override
protected SolrClient getClient(String baseUrl) {
- return httpClient;
+ return solrClient;
}
/**
@@ -115,12 +116,12 @@ public class LBHttp2SolrClient extends LBSolrClient {
@Override
public void setParser(ResponseParser parser) {
super.setParser(parser);
- this.httpClient.setParser(parser);
+ this.solrClient.setParser(parser);
}
@Override
public ResponseParser getParser() {
- return httpClient.getParser();
+ return solrClient.getParser();
}
/**
@@ -136,24 +137,37 @@ public class LBHttp2SolrClient extends LBSolrClient {
@Override
public void setRequestWriter(RequestWriter writer) {
super.setRequestWriter(writer);
- this.httpClient.setRequestWriter(writer);
+ this.solrClient.setRequestWriter(writer);
}
@Override
public RequestWriter getRequestWriter() {
- return httpClient.getRequestWriter();
+ return solrClient.getRequestWriter();
}
- @Override
+ public Set<String> getUrlParamNames() {
+ return solrClient.getUrlParamNames();
+ }
+
+ /**
+ * @deprecated You should instead set this on the passed in Http2SolrClient
used by the Builder.
+ */
+ @Deprecated
public void setQueryParams(Set<String> queryParams) {
- super.setQueryParams(queryParams);
- this.httpClient.setQueryParams(queryParams);
+ this.solrClient.setUrlParamNames(queryParams);
}
- @Override
+ /**
+ * This method should be removed as being able to add a query parameter
isn't compatible with the
+ * idea that query params are an immutable property of a solr client.
+ *
+ * @deprecated you should instead set this on the passed in Http2SolrClient
used by the Builder.
+ */
+ @Deprecated
public void addQueryParams(String queryOnlyParam) {
- super.addQueryParams(queryOnlyParam);
- this.httpClient.setQueryParams(getQueryParams());
+ Set<String> urlParamNames = new
HashSet<>(this.solrClient.getUrlParamNames());
+ urlParamNames.add(queryOnlyParam);
+ this.solrClient.setUrlParamNames(urlParamNames);
}
public Cancellable asyncReq(Req req, AsyncListener<Rsp> asyncListener) {
@@ -301,12 +315,12 @@ public class LBHttp2SolrClient extends LBSolrClient {
public static class Builder {
public static final int CHECK_INTERVAL = 60 * 1000; // 1 minute between
checks
- private final Http2SolrClient http2Client;
+ private final Http2SolrClient http2SolrClient;
private final String[] baseSolrUrls;
private int aliveCheckInterval = CHECK_INTERVAL;
public Builder(Http2SolrClient http2Client, String... baseSolrUrls) {
- this.http2Client = http2Client;
+ this.http2SolrClient = http2Client;
this.baseSolrUrls = baseSolrUrls;
}
@@ -327,7 +341,7 @@ public class LBHttp2SolrClient extends LBSolrClient {
public LBHttp2SolrClient build() {
LBHttp2SolrClient solrClient =
- new LBHttp2SolrClient(this.http2Client,
Arrays.asList(this.baseSolrUrls));
+ new LBHttp2SolrClient(this.http2SolrClient,
Arrays.asList(this.baseSolrUrls));
solrClient.aliveCheckInterval = this.aliveCheckInterval;
return solrClient;
}
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
index 8b321f8e1f8..e89df0c1b93 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
@@ -17,7 +17,9 @@
package org.apache.solr.client.solrj.impl;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.http.client.HttpClient;
import org.apache.solr.client.solrj.SolrClient;
@@ -76,6 +78,7 @@ public class LBHttpSolrClient extends LBSolrClient {
private final boolean clientIsInternal;
private final ConcurrentHashMap<String, HttpSolrClient> urlToClient = new
ConcurrentHashMap<>();
private final HttpSolrClient.Builder httpSolrClientBuilder;
+ private volatile Set<String> urlParamNames = new HashSet<>();
private Integer connectionTimeout;
private volatile Integer soTimeout;
@@ -123,8 +126,8 @@ public class LBHttpSolrClient extends LBSolrClient {
if (requestWriter != null) {
httpSolrClientBuilder.withRequestWriter(requestWriter);
}
- if (queryParams != null) {
- httpSolrClientBuilder.withQueryParams(queryParams);
+ if (urlParamNames != null) {
+ httpSolrClientBuilder.withTheseParamNamesInTheUrl(urlParamNames);
}
client = httpSolrClientBuilder.build();
}
@@ -140,8 +143,8 @@ public class LBHttpSolrClient extends LBSolrClient {
if (requestWriter != null) {
clientBuilder.withRequestWriter(requestWriter);
}
- if (queryParams != null) {
- clientBuilder.withQueryParams(queryParams);
+ if (urlParamNames != null) {
+ clientBuilder.withTheseParamNamesInTheUrl(urlParamNames);
}
client = clientBuilder.build();
}
@@ -178,6 +181,26 @@ public class LBHttpSolrClient extends LBSolrClient {
return httpClient;
}
+ @Deprecated
+ public Set<String> getQueryParams() {
+ return urlParamNames;
+ }
+
+ /**
+ * Expert Method.
+ *
+ * @param urlParamNames set of param keys to only send via the query string
+ */
+ @Deprecated
+ public void setQueryParams(Set<String> urlParamNames) {
+ this.urlParamNames = urlParamNames;
+ }
+
+ @Deprecated
+ public void addQueryParams(String urlParamNames) {
+ this.urlParamNames.add(urlParamNames);
+ }
+
/** Constructs {@link LBHttpSolrClient} instances from provided
configuration. */
public static class Builder extends SolrClientBuilder<Builder> {
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
index 59141fb03f9..5e5f0c0e93a 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
@@ -84,8 +84,6 @@ public abstract class LBSolrClient extends SolrClient {
protected volatile ResponseParser parser;
protected volatile RequestWriter requestWriter;
- protected Set<String> queryParams = new HashSet<>();
-
static {
solrQuery.setRows(0);
/**
@@ -319,23 +317,6 @@ public abstract class LBSolrClient extends SolrClient {
return new ServerWrapper(baseUrl);
}
- public Set<String> getQueryParams() {
- return queryParams;
- }
-
- /**
- * Expert Method.
- *
- * @param queryParams set of param keys to only send via the query string
- */
- public void setQueryParams(Set<String> queryParams) {
- this.queryParams = queryParams;
- }
-
- public void addQueryParams(String queryOnlyParam) {
- this.queryParams.add(queryOnlyParam);
- }
-
public static String normalize(String server) {
if (server.endsWith("/")) server = server.substring(0, server.length() -
1);
return server;
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java
index 43e6ba0e967..a54776c4e78 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java
@@ -35,7 +35,7 @@ public abstract class SolrClientBuilder<B extends
SolrClientBuilder<B>> {
protected Integer connectionTimeoutMillis = 15000;
protected Integer socketTimeoutMillis = 120000;
protected boolean followRedirects = false;
- protected Set<String> queryParams;
+ protected Set<String> urlParamNames;
/** The solution for the unchecked cast warning. */
public abstract B getThis();
@@ -70,8 +70,8 @@ public abstract class SolrClientBuilder<B extends
SolrClientBuilder<B>> {
* @param queryParams set of param keys to only send via the query string
Note that the param will
* be sent as a query string if the key is part of this Set or the
SolrRequest's query params.
*/
- public B withQueryParams(Set<String> queryParams) {
- this.queryParams = queryParams;
+ public B withTheseParamNamesInTheUrl(Set<String> queryParams) {
+ this.urlParamNames = queryParams;
return getThis();
}
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java
index 11d59297d3d..79a22c29e6c 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java
@@ -29,7 +29,6 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.TreeSet;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -747,14 +746,6 @@ public class BasicHttpSolrClientTest extends
SolrJettyTestBase {
}
}
- private Set<String> setOf(String... keys) {
- Set<String> set = new TreeSet<>();
- if (keys != null) {
- Collections.addAll(set, keys);
- }
- return set;
- }
-
private void setReqParamsOf(UpdateRequest req, String... keys) {
if (keys != null) {
for (String k : keys) {
@@ -790,7 +781,8 @@ public class BasicHttpSolrClientTest extends
SolrJettyTestBase {
final String clientUrl = jetty.getBaseUrl().toString() + "/debug/foo";
HttpSolrClient.Builder builder = new HttpSolrClient.Builder(clientUrl);
- try (HttpSolrClient client =
builder.withQueryParams(setOf("serverOnly")).build()) {
+ try (HttpSolrClient client =
+ builder.withTheseParamNamesInTheUrl(Set.of("serverOnly")).build()) {
// test without request query params
DebugServlet.clear();
UpdateRequest req = new UpdateRequest();
@@ -798,36 +790,38 @@ public class BasicHttpSolrClientTest extends
SolrJettyTestBase {
expectThrows(BaseHttpSolrClient.RemoteSolrException.class, () ->
client.request(req));
verifyServletState(client, req);
}
- try (HttpSolrClient client = builder.withQueryParams(setOf()).build()) {
+ try (HttpSolrClient client =
builder.withTheseParamNamesInTheUrl(Set.of()).build()) {
// test without server query params
DebugServlet.clear();
UpdateRequest req2 = new UpdateRequest();
- req2.setQueryParams(setOf("requestOnly"));
+ req2.setQueryParams(Set.of("requestOnly"));
setReqParamsOf(req2, "requestOnly", "notRequest");
expectThrows(BaseHttpSolrClient.RemoteSolrException.class, () ->
client.request(req2));
verifyServletState(client, req2);
}
- try (HttpSolrClient client = builder.withQueryParams(setOf("serverOnly",
"both")).build()) {
+ try (HttpSolrClient client =
+ builder.withTheseParamNamesInTheUrl(Set.of("serverOnly",
"both")).build()) {
// test with both request and server query params
DebugServlet.clear();
UpdateRequest req3 = new UpdateRequest();
- req3.setQueryParams(setOf("requestOnly", "both"));
+ req3.setQueryParams(Set.of("requestOnly", "both"));
setReqParamsOf(req3, "serverOnly", "requestOnly", "both", "neither");
expectThrows(BaseHttpSolrClient.RemoteSolrException.class, () ->
client.request(req3));
verifyServletState(client, req3);
}
- try (HttpSolrClient client = builder.withQueryParams(setOf("serverOnly",
"both")).build()) {
+ try (HttpSolrClient client =
+ builder.withTheseParamNamesInTheUrl(Set.of("serverOnly",
"both")).build()) {
// test with both request and server query params with single stream
DebugServlet.clear();
UpdateRequest req4 = new UpdateRequest();
req4.add(new SolrInputDocument());
- req4.setQueryParams(setOf("requestOnly", "both"));
+ req4.setQueryParams(Set.of("requestOnly", "both"));
setReqParamsOf(req4, "serverOnly", "requestOnly", "both", "neither");
expectThrows(BaseHttpSolrClient.RemoteSolrException.class, () ->
client.request(req4));
// NOTE: single stream requests send all the params
// as part of the query string. So add "neither" to the request
// so it passes the verification step.
- req4.setQueryParams(setOf("requestOnly", "both", "neither"));
+ req4.setQueryParams(Set.of("requestOnly", "both", "neither"));
verifyServletState(client, req4);
}
}
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
index e08b12f8a3f..57636345077 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
@@ -30,7 +30,6 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
-import java.util.TreeSet;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -189,7 +188,7 @@ public class Http2SolrClientTest extends SolrJettyTestBase {
super.tearDown();
}
- private Http2SolrClient.Builder getHttp2SolrClient(
+ private Http2SolrClient.Builder getHttp2SolrClientBuilder(
String url, int connectionTimeOut, int socketTimeout) {
return new Http2SolrClient.Builder(url)
.connectionTimeout(connectionTimeOut)
@@ -200,7 +199,7 @@ public class Http2SolrClientTest extends SolrJettyTestBase {
public void testTimeout() throws Exception {
SolrQuery q = new SolrQuery("*:*");
try (Http2SolrClient client =
- getHttp2SolrClient(
+ getHttp2SolrClientBuilder(
jetty.getBaseUrl().toString() + "/slow/foo",
DEFAULT_CONNECTION_TIMEOUT, 2000)
.build()) {
client.query(q, SolrRequest.METHOD.GET);
@@ -214,7 +213,7 @@ public class Http2SolrClientTest extends SolrJettyTestBase {
public void test0IdleTimeout() throws Exception {
SolrQuery q = new SolrQuery("*:*");
try (Http2SolrClient client =
- getHttp2SolrClient(
+ getHttp2SolrClientBuilder(
jetty.getBaseUrl().toString() + "/debug/foo",
DEFAULT_CONNECTION_TIMEOUT, 0)
.build()) {
try {
@@ -228,7 +227,7 @@ public class Http2SolrClientTest extends SolrJettyTestBase {
public void testRequestTimeout() throws Exception {
SolrQuery q = new SolrQuery("*:*");
try (Http2SolrClient client =
- getHttp2SolrClient(
+ getHttp2SolrClientBuilder(
jetty.getBaseUrl().toString() + "/slow/foo",
DEFAULT_CONNECTION_TIMEOUT, 0)
.requestTimeout(500)
.build()) {
@@ -619,14 +618,6 @@ public class Http2SolrClientTest extends SolrJettyTestBase
{
}
}
- private Set<String> setOf(String... keys) {
- Set<String> set = new TreeSet<>();
- if (keys != null) {
- Collections.addAll(set, keys);
- }
- return set;
- }
-
private void setReqParamsOf(UpdateRequest req, String... keys) {
if (keys != null) {
for (String k : keys) {
@@ -644,7 +635,7 @@ public class Http2SolrClientTest extends SolrJettyTestBase {
if (values != null) {
for (String value : values) {
boolean shouldBeInQueryString =
- client.getQueryParams().contains(name)
+ client.getUrlParamNames().contains(name)
|| (request.getQueryParams() != null &&
request.getQueryParams().contains(name));
assertEquals(
shouldBeInQueryString, DebugServlet.queryString.contains(name +
"=" + value));
@@ -661,12 +652,16 @@ public class Http2SolrClientTest extends
SolrJettyTestBase {
public void testQueryString() throws Exception {
final String clientUrl = jetty.getBaseUrl().toString() + "/debug/foo";
- try (Http2SolrClient client = getHttp2SolrClient(clientUrl)) {
+ UpdateRequest req = new UpdateRequest();
+
+ try (Http2SolrClient client =
+ new Http2SolrClient.Builder(clientUrl)
+ .withTheseParamNamesInTheUrl(Set.of("serverOnly"))
+ .build()) {
// test without request query params
DebugServlet.clear();
- client.setQueryParams(setOf("serverOnly"));
- UpdateRequest req = new UpdateRequest();
setReqParamsOf(req, "serverOnly", "notServer");
+
try {
client.request(req);
} catch (BaseHttpSolrClient.RemoteSolrException ignored) {
@@ -675,9 +670,11 @@ public class Http2SolrClientTest extends SolrJettyTestBase
{
// test without server query params
DebugServlet.clear();
- client.setQueryParams(setOf());
+ }
+ try (Http2SolrClient client =
+ new
Http2SolrClient.Builder(clientUrl).withTheseParamNamesInTheUrl(Set.of()).build())
{
req = new UpdateRequest();
- req.setQueryParams(setOf("requestOnly"));
+ req.setQueryParams(Set.of("requestOnly"));
setReqParamsOf(req, "requestOnly", "notRequest");
try {
client.request(req);
@@ -687,22 +684,30 @@ public class Http2SolrClientTest extends
SolrJettyTestBase {
// test with both request and server query params
DebugServlet.clear();
+ }
+ try (Http2SolrClient client =
+ new Http2SolrClient.Builder(clientUrl)
+ .withTheseParamNamesInTheUrl(Set.of("serverOnly", "both"))
+ .build()) {
req = new UpdateRequest();
- client.setQueryParams(setOf("serverOnly", "both"));
- req.setQueryParams(setOf("requestOnly", "both"));
+ req.setQueryParams(Set.of("requestOnly", "both"));
setReqParamsOf(req, "serverOnly", "requestOnly", "both", "neither");
try {
client.request(req);
} catch (BaseHttpSolrClient.RemoteSolrException ignored) {
}
verifyServletState(client, req);
+ }
+ try (Http2SolrClient client =
+ new Http2SolrClient.Builder(clientUrl)
+ .withTheseParamNamesInTheUrl(Set.of("serverOnly", "both"))
+ .build()) {
// test with both request and server query params with single stream
DebugServlet.clear();
req = new UpdateRequest();
req.add(new SolrInputDocument());
- client.setQueryParams(setOf("serverOnly", "both"));
- req.setQueryParams(setOf("requestOnly", "both"));
+ req.setQueryParams(Set.of("requestOnly", "both"));
setReqParamsOf(req, "serverOnly", "requestOnly", "both", "neither");
try {
client.request(req);
@@ -711,7 +716,7 @@ public class Http2SolrClientTest extends SolrJettyTestBase {
// NOTE: single stream requests send all the params
// as part of the query string. So add "neither" to the request
// so it passes the verification step.
- req.setQueryParams(setOf("requestOnly", "both", "neither"));
+ req.setQueryParams(Set.of("requestOnly", "both", "neither"));
verifyServletState(client, req);
}
}
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
index fa275250056..8e1018a66b6 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
@@ -16,7 +16,6 @@
*/
package org.apache.solr.client.solrj.impl;
-import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import org.apache.solr.SolrTestCase;
@@ -26,39 +25,43 @@ import org.junit.Test;
public class LBHttp2SolrClientTest extends SolrTestCase {
/**
- * Test method for {@link LBHttp2SolrClient#setQueryParams(Set)} and {@link
- * LBHttp2SolrClient#addQueryParams(String)}.
+ * Test method for {@link LBHttp2SolrClient.Builder} that validates that the
query param keys
+ * passed in by the base <code>Http2SolrClient
+ * </code> instance are used by the LBHttp2SolrClient.
*
- * <p>Validate that the query params passed in are used in the base
<code>Http2SolrClient</code>
- * instance.
+ * <p>TODO: Eliminate the addQueryParams aspect of test as it is not
compatible with goal of
+ * immutable SolrClient
*/
@Test
- public void testLBHttp2SolrClientSetQueryParams() throws IOException {
+ public void testLBHttp2SolrClientSetQueryParams() {
String url = "http://127.0.0.1:8080";;
- try (Http2SolrClient http2Client = new
Http2SolrClient.Builder(url).build();
- LBHttp2SolrClient testClient = new
LBHttp2SolrClient.Builder(http2Client, url).build()) {
- Set<String> queryParams = new HashSet<>(2);
- queryParams.add("param1=this");
- testClient.setQueryParams(new HashSet<>(queryParams));
+ Set<String> urlParamNames = new HashSet<>(2);
+ urlParamNames.add("param1");
+
+ try (Http2SolrClient http2SolrClient =
+ new
Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames).build();
+ LBHttp2SolrClient testClient =
+ new LBHttp2SolrClient.Builder(http2SolrClient, url).build()) {
+
assertArrayEquals(
- "Wrong queryParams found in lb client.",
- queryParams.toArray(),
- testClient.getQueryParams().toArray());
+ "Wrong urlParamNames found in lb client.",
+ urlParamNames.toArray(),
+ testClient.getUrlParamNames().toArray());
assertArrayEquals(
- "Wrong queryParams found in base client.",
- queryParams.toArray(),
- http2Client.getQueryParams().toArray());
+ "Wrong urlParamNames found in base client.",
+ urlParamNames.toArray(),
+ http2SolrClient.getUrlParamNames().toArray());
- testClient.addQueryParams("param2=that");
- queryParams.add("param2=that");
+ testClient.addQueryParams("param2");
+ urlParamNames.add("param2");
assertArrayEquals(
- "Wrong queryParams found in lb client.",
- queryParams.toArray(),
- testClient.getQueryParams().toArray());
+ "Wrong urlParamNames found in lb client.",
+ urlParamNames.toArray(),
+ testClient.getUrlParamNames().toArray());
assertArrayEquals(
- "Wrong queryParams found in base client.",
- queryParams.toArray(),
- http2Client.getQueryParams().toArray());
+ "Wrong urlParamNames found in base client.",
+ urlParamNames.toArray(),
+ http2SolrClient.getUrlParamNames().toArray());
}
}
}
