This is an automated email from the ASF dual-hosted git repository.
tflobbe pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 015f8a22449 SOLR-16737 Http2SolrClient needs to inherit all properties
when initialized with another http2 client (#1587)
015f8a22449 is described below
commit 015f8a22449aab1cca6ef2e963fbe172c59d5e44
Author: Alex <[email protected]>
AuthorDate: Wed Apr 26 09:47:42 2023 -0700
SOLR-16737 Http2SolrClient needs to inherit all properties when initialized
with another http2 client (#1587)
---
solr/CHANGES.txt | 2 +
.../client/solrj/impl/CloudHttp2SolrClient.java | 2 +-
.../solr/client/solrj/impl/Http2SolrClient.java | 106 +++++++++++++--------
.../client/solrj/impl/Http2SolrClientTest.java | 35 +++++++
4 files changed, 105 insertions(+), 40 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index dcdd1a7fd08..1566db19ce6 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -152,6 +152,8 @@ Bug Fixes
* SOLR-7609: Internal update requests should fail back to the client in some
edge cases for shard splits. Use HTTP status 510 so the client can retry the
operation. (Alex Deparvu, David Smiley, Tomás Fernández Löbbe)
+* SOLR-16737: Http2SolrClient needs to inherit all properties when initialized
with another http2 client (Alex Deparvu, Tomás Fernández Löbbe)
+
Dependency Upgrades
---------------------
* PR#1494: Upgrade forbiddenapis to 3.5 (Uwe Schindler)
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
index 1da2c841955..5222275c069 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java
@@ -71,7 +71,7 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
this.myClient.requestWriter = builder.requestWriter;
}
if (builder.responseParser != null) {
- this.myClient.parser = builder.responseParser;
+ this.myClient.setParser(builder.responseParser);
}
if (builder.stateProvider == null) {
if (builder.zkHosts != null && builder.solrUrls != null) {
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 50075e2abce..c088c1390fd 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
@@ -131,25 +131,26 @@ public class Http2SolrClient extends SolrClient {
private static final String DEFAULT_PATH = "/select";
private static final List<String> errPath = Arrays.asList("metadata",
"error-class");
- private HttpClient httpClient;
- private volatile Set<String> urlParamNames = Set.of();
- private long idleTimeoutMillis;
- private long requestTimeoutMillis;
+ private final HttpClient httpClient;
+ private final Set<String> urlParamNames;
+ private final long idleTimeoutMillis;
+ private final long requestTimeoutMillis;
- protected ResponseParser parser = new BinaryResponseParser();
+ // updating parser instance needs to go via the setter to ensure update of
defaultParserMimeTypes
+ private ResponseParser parser = new BinaryResponseParser();
private Set<String> defaultParserMimeTypes;
protected RequestWriter requestWriter = new BinaryRequestWriter();
private List<HttpListenerFactory> listenerFactory = new ArrayList<>();
- private AsyncTracker asyncTracker = new AsyncTracker();
+ private final AsyncTracker asyncTracker = new AsyncTracker();
/** The URL of the Solr server. */
private final String serverBaseUrl;
- private boolean closeClient;
+ private final boolean closeClient;
private ExecutorService executor;
private boolean shutdownExecutor;
- private final String basicAuthAuthorizationStr;
+ final String basicAuthAuthorizationStr;
private AuthenticationStoreHolder authenticationStore;
protected Http2SolrClient(String serverBaseUrl, Builder builder) {
@@ -167,38 +168,37 @@ public class Http2SolrClient extends SolrClient {
}
if (builder.idleTimeoutMillis != null && builder.idleTimeoutMillis > 0) {
- idleTimeoutMillis = builder.idleTimeoutMillis;
+ this.idleTimeoutMillis = builder.idleTimeoutMillis;
} else {
- idleTimeoutMillis = HttpClientUtil.DEFAULT_SO_TIMEOUT;
+ this.idleTimeoutMillis = HttpClientUtil.DEFAULT_SO_TIMEOUT;
}
- if (builder.http2SolrClient == null) {
- httpClient = createHttpClient(builder);
- closeClient = true;
+ if (builder.httpClient != null) {
+ this.httpClient = builder.httpClient;
+ this.closeClient = false;
} else {
- httpClient = builder.http2SolrClient.httpClient;
- }
- if (builder.basicAuthUser != null && builder.basicAuthPassword != null) {
- basicAuthAuthorizationStr =
- basicAuthCredentialsToAuthorizationString(
- builder.basicAuthUser, builder.basicAuthPassword);
- } else {
- basicAuthAuthorizationStr = null;
+ this.httpClient = createHttpClient(builder);
+ this.closeClient = true;
}
+ this.basicAuthAuthorizationStr = builder.basicAuthAuthorizationStr;
if (builder.requestWriter != null) {
- requestWriter = builder.requestWriter;
+ this.requestWriter = builder.requestWriter;
}
if (builder.responseParser != null) {
- parser = builder.responseParser;
+ this.parser = builder.responseParser;
}
updateDefaultMimeTypeForParser();
- if (builder.requestTimeoutMillis == null) {
- requestTimeoutMillis = -1;
+ if (builder.requestTimeoutMillis != null) {
+ this.requestTimeoutMillis = builder.requestTimeoutMillis;
+ } else {
+ this.requestTimeoutMillis = -1;
+ }
+
this.httpClient.setFollowRedirects(Boolean.TRUE.equals(builder.followRedirects));
+ if (builder.urlParamNames != null) {
+ this.urlParamNames = builder.urlParamNames;
} else {
- requestTimeoutMillis = builder.requestTimeoutMillis;
+ this.urlParamNames = Set.of();
}
- httpClient.setFollowRedirects(builder.followRedirects);
- this.urlParamNames = builder.urlParamNames;
assert ObjectReleaseTracker.track(this);
}
@@ -572,7 +572,7 @@ public class Http2SolrClient extends SolrClient {
}
}
- private String basicAuthCredentialsToAuthorizationString(String user, String
pass) {
+ static String basicAuthCredentialsToAuthorizationString(String user, String
pass) {
String userPass = user + ":" + pass;
return "Basic " +
Base64.getEncoder().encodeToString(userPass.getBytes(FALLBACK_CHARSET));
}
@@ -998,21 +998,20 @@ public class Http2SolrClient extends SolrClient {
public static class Builder {
- private Http2SolrClient http2SolrClient;
+ private HttpClient httpClient;
private SSLConfig sslConfig = defaultSSLConfig;
private Long idleTimeoutMillis;
private Long connectionTimeoutMillis;
private Long requestTimeoutMillis;
private Integer maxConnectionsPerHost;
- private String basicAuthUser;
- private String basicAuthPassword;
+ private String basicAuthAuthorizationStr;
private boolean useHttp1_1 = Boolean.getBoolean("solr.http1");
- private boolean followRedirects = false;
+ private Boolean followRedirects;
protected String baseSolrUrl;
private ExecutorService executor;
protected RequestWriter requestWriter;
protected ResponseParser responseParser;
- private Set<String> urlParamNames = Set.of();
+ private Set<String> urlParamNames;
private CookieStore cookieStore = getDefaultCookieStore();
public Builder() {}
@@ -1068,9 +1067,34 @@ public class Http2SolrClient extends SolrClient {
return null;
}
- /** Reuse {@code httpClient} connections pool */
- public Builder withHttpClient(Http2SolrClient httpClient) {
- this.http2SolrClient = httpClient;
+ /**
+ * Provide a seed Http2SolrClient for the builder values, values can still
be overridden by
+ * using builder methods
+ */
+ public Builder withHttpClient(Http2SolrClient http2SolrClient) {
+ this.httpClient = http2SolrClient.httpClient;
+
+ if (this.basicAuthAuthorizationStr == null) {
+ this.basicAuthAuthorizationStr =
http2SolrClient.basicAuthAuthorizationStr;
+ }
+ if (this.followRedirects == null) {
+ this.followRedirects = http2SolrClient.httpClient.isFollowRedirects();
+ }
+ if (this.idleTimeoutMillis == null) {
+ this.idleTimeoutMillis = http2SolrClient.idleTimeoutMillis;
+ }
+ if (this.requestWriter == null) {
+ this.requestWriter = http2SolrClient.requestWriter;
+ }
+ if (this.requestTimeoutMillis == null) {
+ this.requestTimeoutMillis = http2SolrClient.requestTimeoutMillis;
+ }
+ if (this.responseParser == null) {
+ this.responseParser = http2SolrClient.parser;
+ }
+ if (this.urlParamNames == null) {
+ this.urlParamNames = http2SolrClient.urlParamNames;
+ }
return this;
}
@@ -1108,8 +1132,7 @@ public class Http2SolrClient extends SolrClient {
"Invalid Authentication credentials. Either both username and
password or none must be provided");
}
}
- this.basicAuthUser = user;
- this.basicAuthPassword = pass;
+ this.basicAuthAuthorizationStr =
basicAuthCredentialsToAuthorizationString(user, pass);
return this;
}
@@ -1240,6 +1263,11 @@ public class Http2SolrClient extends SolrClient {
return parser;
}
+ protected void setParser(ResponseParser parser) {
+ this.parser = parser;
+ updateDefaultMimeTypeForParser();
+ }
+
protected void updateDefaultMimeTypeForParser() {
defaultParserMimeTypes =
parser.getContentTypes().stream()
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 10ac4b345ce..c8aaf736544 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
@@ -956,5 +956,40 @@ public class Http2SolrClientTest extends SolrJettyTestBase
{
}
}
+ @Test
+ public void testBuilder() {
+ try (Http2SolrClient seed =
+ new Http2SolrClient.Builder("baseSolrUrl")
+ .withBasicAuthCredentials("testu", "testp")
+ .build()) {
+
+ Http2SolrClient clone1 =
+ new
Http2SolrClient.Builder("baseSolrUrl").withHttpClient(seed).build();
+ String expected1 =
+ Http2SolrClient.basicAuthCredentialsToAuthorizationString("testu",
"testp");
+ assertEquals(expected1, clone1.basicAuthAuthorizationStr);
+
+ // test overwrite seed value
+ Http2SolrClient clone2 =
+ new Http2SolrClient.Builder("baseSolrUrl")
+ .withHttpClient(seed)
+ .withBasicAuthCredentials("testu2", "testp2")
+ .build();
+ String expected2 =
+ Http2SolrClient.basicAuthCredentialsToAuthorizationString("testu2",
"testp2");
+ assertEquals(expected2, clone2.basicAuthAuthorizationStr);
+
+ // test overwrite seed value, order of builder method calls reversed
+ Http2SolrClient clone3 =
+ new Http2SolrClient.Builder("baseSolrUrl")
+ .withBasicAuthCredentials("testu3", "testp3")
+ .withHttpClient(seed)
+ .build();
+ String expected3 =
+ Http2SolrClient.basicAuthCredentialsToAuthorizationString("testu3",
"testp3");
+ assertEquals(expected3, clone3.basicAuthAuthorizationStr);
+ }
+ }
+
/** Missed tests : - set cookies via interceptor - invariant params -
compression */
}
