You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ro...@apache.org on 2016/09/12 08:22:15 UTC
lucene-solr:branch_6x: SOLR-9344: Allow multiple
HttpClientConfigurers to be set on HttpClientUtil
Repository: lucene-solr
Updated Branches:
refs/heads/branch_6x b5f5bb10a -> 66a51b76b
SOLR-9344: Allow multiple HttpClientConfigurers to be set on HttpClientUtil
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/66a51b76
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/66a51b76
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/66a51b76
Branch: refs/heads/branch_6x
Commit: 66a51b76b399ac0b018a4f600de3fc349ae7a2ec
Parents: b5f5bb1
Author: Alan Woodward <ro...@apache.org>
Authored: Sat Sep 10 22:35:03 2016 +0100
Committer: Alan Woodward <ro...@apache.org>
Committed: Mon Sep 12 09:08:35 2016 +0100
----------------------------------------------------------------------
solr/CHANGES.txt | 5 ++
.../org/apache/solr/core/CoreContainer.java | 2 +-
.../solr/cloud/TestMiniSolrCloudClusterSSL.java | 51 ++++++++----------
.../solr/client/solrj/impl/HttpClientUtil.java | 44 ++++++++++++----
.../client/solrj/impl/HttpClientUtilTest.java | 54 ++++++++++++--------
.../java/org/apache/solr/SolrTestCaseJ4.java | 5 +-
6 files changed, 99 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/66a51b76/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 849d1b9..11b845d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -88,6 +88,11 @@ Bug Fixes
* SOLR-9490: Fixed bugs in BoolField that caused it to erroneously return "false" for all docs depending
on usage (Colvin Cowie, Dan Fox, hossman)
+* SOLR-9344: HttpClientConfigurers are now chained, rather than only one being
+ set at a time. This prevents a bug where BasicAuth and SSL could not be
+ combined on an HttpClient object. (Alan Woodward)
+
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/66a51b76/solr/core/src/java/org/apache/solr/core/CoreContainer.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index 4567751..16dce9c 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -327,7 +327,7 @@ public class CoreContainer {
if (authcPlugin instanceof HttpClientInterceptorPlugin) {
// Setup HttpClient to use the plugin's configurer for internode communication
HttpClientConfigurer configurer = ((HttpClientInterceptorPlugin) authcPlugin).getClientConfigurer();
- HttpClientUtil.setConfigurer(configurer);
+ HttpClientUtil.addConfigurer(configurer);
// The default http client of the core container's shardHandlerFactory has already been created and
// configured using the default httpclient configurer. We need to reconfigure it using the plugin's
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/66a51b76/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
index a011162..b79e8f9 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java
@@ -16,43 +16,36 @@
*/
package org.apache.solr.cloud;
+import javax.net.ssl.SSLContext;
+import java.io.File;
+import java.io.IOException;
import java.lang.invoke.MethodHandles;
-
import java.util.Collections;
import java.util.List;
-import java.io.File;
-import java.io.IOException;
-
-import javax.net.ssl.SSLContext;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpHead;
+import org.apache.http.config.Registry;
+import org.apache.http.config.RegistryBuilder;
+import org.apache.http.conn.socket.ConnectionSocketFactory;
+import org.apache.http.conn.socket.PlainConnectionSocketFactory;
+import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.lucene.util.Constants;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.embedded.JettyConfig;
import org.apache.solr.client.solrj.embedded.JettySolrRunner;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
-import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.impl.HttpClientUtil;
-import org.apache.solr.client.solrj.impl.HttpClientConfigurer;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.request.CoreAdminRequest;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction;
import org.apache.solr.util.SSLTestConfig;
-
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClientBuilder;
-import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpHead;
-import org.apache.http.config.RegistryBuilder;
-import org.apache.http.config.Registry;
-import org.apache.http.conn.socket.ConnectionSocketFactory;
-import org.apache.http.conn.socket.PlainConnectionSocketFactory;
-import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
-
-import org.apache.lucene.util.Constants;
-
import org.junit.After;
import org.junit.Before;
-
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -83,19 +76,19 @@ public class TestMiniSolrCloudClusterSSL extends SolrTestCaseJ4 {
public void before() {
// undo the randomization of our super class
log.info("NOTE: This Test ignores the randomized SSL & clientAuth settings selected by base class");
- HttpClientUtil.setConfigurer(new HttpClientConfigurer());
+ HttpClientUtil.resetConfigurers();
System.clearProperty(ZkStateReader.URL_SCHEME);
}
@After
public void after() {
- HttpClientUtil.setConfigurer(new HttpClientConfigurer());
+ HttpClientUtil.resetConfigurers();
System.clearProperty(ZkStateReader.URL_SCHEME);
SSLContext.setDefault(DEFAULT_SSL_CONTEXT);
}
public void testNoSsl() throws Exception {
final SSLTestConfig sslConfig = new SSLTestConfig(false, false);
- HttpClientUtil.setConfigurer(sslConfig.getHttpClientConfigurer());
+ HttpClientUtil.addConfigurer(sslConfig.getHttpClientConfigurer());
System.setProperty(ZkStateReader.URL_SCHEME, "http");
checkClusterWithNodeReplacement(sslConfig);
}
@@ -105,14 +98,14 @@ public class TestMiniSolrCloudClusterSSL extends SolrTestCaseJ4 {
// but we test it anyway for completeness of sanity checking the behavior of code that looks at those
// options.
final SSLTestConfig sslConfig = new SSLTestConfig(false, true);
- HttpClientUtil.setConfigurer(sslConfig.getHttpClientConfigurer());
+ HttpClientUtil.addConfigurer(sslConfig.getHttpClientConfigurer());
System.setProperty(ZkStateReader.URL_SCHEME, "http");
checkClusterWithNodeReplacement(sslConfig);
}
public void testSslAndNoClientAuth() throws Exception {
final SSLTestConfig sslConfig = new SSLTestConfig(true, false);
- HttpClientUtil.setConfigurer(sslConfig.getHttpClientConfigurer());
+ HttpClientUtil.addConfigurer(sslConfig.getHttpClientConfigurer());
System.setProperty(ZkStateReader.URL_SCHEME, "https");
checkClusterWithNodeReplacement(sslConfig);
}
@@ -122,7 +115,7 @@ public class TestMiniSolrCloudClusterSSL extends SolrTestCaseJ4 {
final SSLTestConfig sslConfig = new SSLTestConfig(true, true);
- HttpClientUtil.setConfigurer(sslConfig.getHttpClientConfigurer());
+ HttpClientUtil.addConfigurer(sslConfig.getHttpClientConfigurer());
System.setProperty(ZkStateReader.URL_SCHEME, "https");
checkClusterWithNodeReplacement(sslConfig);
}
@@ -149,7 +142,7 @@ public class TestMiniSolrCloudClusterSSL extends SolrTestCaseJ4 {
// our test config doesn't use SSL, and reset HttpClientUtil to it's defaults so it picks up our
// SSLContext that way.
SSLContext.setDefault( sslConfig.isSSLMode() ? sslConfig.buildClientSSLContext() : DEFAULT_SSL_CONTEXT);
- HttpClientUtil.setConfigurer(new HttpClientConfigurer());
+ HttpClientUtil.resetConfigurers();
// recheck that we can communicate with all the jetty instances in our cluster
checkClusterJettys(cluster, sslConfig);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/66a51b76/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java
index 2ad8975..96f4d82 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java
@@ -27,6 +27,7 @@ import java.util.List;
import java.util.zip.GZIPInputStream;
import java.util.zip.InflaterInputStream;
+import com.google.common.collect.Lists;
import org.apache.http.Header;
import org.apache.http.HeaderElement;
import org.apache.http.HttpEntity;
@@ -93,20 +94,41 @@ public class HttpClientUtil {
static final DefaultHttpRequestRetryHandler NO_RETRY = new DefaultHttpRequestRetryHandler(
0, false);
- private static HttpClientConfigurer configurer = new HttpClientConfigurer();
+ private static final List<HttpClientConfigurer> configurers
+ = Collections.synchronizedList(Lists.newArrayList(new HttpClientConfigurer()));
- private static final List<HttpRequestInterceptor> interceptors = Collections.synchronizedList(new ArrayList<HttpRequestInterceptor>());
+ private static final List<HttpRequestInterceptor> interceptors = Collections.synchronizedList(new ArrayList<>());
/**
- * Replace the {@link HttpClientConfigurer} class used in configuring the http
- * clients with a custom implementation.
+ * Add a {@link HttpClientConfigurer} class used to configure the http
+ * clients.
+ *
+ * @deprecated use {@link #addConfigurer(HttpClientConfigurer)}
*/
+ @Deprecated
public static void setConfigurer(HttpClientConfigurer newConfigurer) {
- configurer = newConfigurer;
+ addConfigurer(newConfigurer);
}
-
- public static HttpClientConfigurer getConfigurer() {
- return configurer;
+
+ /**
+ * Add a {@link HttpClientConfigurer} to the list of configurers used
+ * to create http clients
+ */
+ public static void addConfigurer(HttpClientConfigurer newConfigurer) {
+ configurers.add(newConfigurer);
+ }
+
+ /**
+ * Remove a {@link HttpClientConfigurer} from the list of configurers used
+ * to create http clients
+ */
+ public static void removeConfigurer(HttpClientConfigurer configurer) {
+ configurers.remove(configurer);
+ }
+
+ public static void resetConfigurers() {
+ configurers.clear();
+ configurers.add(new HttpClientConfigurer());
}
/**
@@ -146,7 +168,11 @@ public class HttpClientUtil {
*/
public static void configureClient(final DefaultHttpClient httpClient,
SolrParams config) {
- configurer.configure(httpClient, config);
+ synchronized (configurers) {
+ for (HttpClientConfigurer configurer : configurers) {
+ configurer.configure(httpClient, config);
+ }
+ }
synchronized(interceptors) {
for(HttpRequestInterceptor interceptor: interceptors) {
httpClient.addRequestInterceptor(interceptor);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/66a51b76/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClientUtilTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClientUtilTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClientUtilTest.java
index 9fa2b0b..a882623 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClientUtilTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClientUtilTest.java
@@ -47,6 +47,7 @@ public class HttpClientUtilTest extends LuceneTestCase {
client.close();
}
+ @SuppressWarnings("deprecation")
@Test
public void testSetParams() {
ModifiableSolrParams params = new ModifiableSolrParams();
@@ -59,27 +60,25 @@ public class HttpClientUtilTest extends LuceneTestCase {
params.set(HttpClientUtil.PROP_MAX_CONNECTIONS_PER_HOST, 32345);
params.set(HttpClientUtil.PROP_SO_TIMEOUT, 42345);
params.set(HttpClientUtil.PROP_USE_RETRY, false);
- DefaultHttpClient client = (DefaultHttpClient) HttpClientUtil.createClient(params);
- try {
+ try (DefaultHttpClient client = (DefaultHttpClient) HttpClientUtil.createClient(params)) {
assertEquals(12345, HttpConnectionParams.getConnectionTimeout(client.getParams()));
assertEquals(PoolingClientConnectionManager.class, client.getConnectionManager().getClass());
- assertEquals(22345, ((PoolingClientConnectionManager)client.getConnectionManager()).getMaxTotal());
- assertEquals(32345, ((PoolingClientConnectionManager)client.getConnectionManager()).getDefaultMaxPerRoute());
+ assertEquals(22345, ((PoolingClientConnectionManager) client.getConnectionManager()).getMaxTotal());
+ assertEquals(32345, ((PoolingClientConnectionManager) client.getConnectionManager()).getDefaultMaxPerRoute());
assertEquals(42345, HttpConnectionParams.getSoTimeout(client.getParams()));
assertEquals(HttpClientUtil.NO_RETRY, client.getHttpRequestRetryHandler());
assertEquals("pass", client.getCredentialsProvider().getCredentials(new AuthScope("127.0.0.1", 1234)).getPassword());
assertEquals("user", client.getCredentialsProvider().getCredentials(new AuthScope("127.0.0.1", 1234)).getUserPrincipal().getName());
assertEquals(true, client.getParams().getParameter(ClientPNames.HANDLE_REDIRECTS));
- } finally {
- client.close();
}
}
+ @SuppressWarnings("deprecation")
@Test
public void testAuthSchemeConfiguration() {
System.setProperty(Krb5HttpClientConfigurer.LOGIN_CONFIG_PROP, "test");
try {
- HttpClientUtil.setConfigurer(new Krb5HttpClientConfigurer());
+ HttpClientUtil.addConfigurer(new Krb5HttpClientConfigurer());
AbstractHttpClient client = (AbstractHttpClient)HttpClientUtil.createClient(null);
assertEquals(1, client.getAuthSchemes().getSchemeNames().size());
assertTrue(AuthSchemes.SPNEGO.equalsIgnoreCase(client.getAuthSchemes().getSchemeNames().get(0)));
@@ -89,30 +88,45 @@ public class HttpClientUtilTest extends LuceneTestCase {
}
}
+ @SuppressWarnings("deprecation")
@Test
- public void testReplaceConfigurer() throws IOException{
-
- try {
- final AtomicInteger counter = new AtomicInteger();
- HttpClientConfigurer custom = new HttpClientConfigurer(){
+ public void testMultipleConfigurers() throws IOException {
+
+ final AtomicInteger counter1 = new AtomicInteger();
+ final AtomicInteger counter2 = new AtomicInteger();
+
+ HttpClientConfigurer custom1 = new HttpClientConfigurer() {
@Override
public void configure(DefaultHttpClient httpClient, SolrParams config) {
super.configure(httpClient, config);
- counter.set(config.getInt("custom-param", -1));
+ counter1.set(config.getInt("custom-param", -1));
+ }
+ };
+ HttpClientConfigurer custom2 = new HttpClientConfigurer() {
+ @Override
+ public void configure(DefaultHttpClient httpClient, SolrParams config) {
+ super.configure(httpClient, config);
+ counter2.set(config.getInt("custom-param", -1));
}
-
};
- HttpClientUtil.setConfigurer(custom);
+ HttpClientUtil.addConfigurer(custom1);
+ HttpClientUtil.addConfigurer(custom2);
ModifiableSolrParams params = new ModifiableSolrParams();
params.set("custom-param", 5);
HttpClientUtil.createClient(params).close();
- assertEquals(5, counter.get());
- } finally {
- //restore default configurer
- HttpClientUtil.setConfigurer(new HttpClientConfigurer());
- }
+ assertEquals(5, counter1.get());
+ assertEquals(5, counter2.get());
+
+ HttpClientUtil.createClient(null).close();
+ assertEquals(-1, counter1.get());
+ assertEquals(-1, counter2.get());
+
+ HttpClientUtil.removeConfigurer(custom1);
+ HttpClientUtil.createClient(params).close();
+ assertEquals(-1, counter1.get());
+ assertEquals(5, counter2.get());
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/66a51b76/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
----------------------------------------------------------------------
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
index 2383a3c..c15da95 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
@@ -67,15 +67,14 @@ import org.apache.lucene.util.LuceneTestCase.SuppressFileSystems;
import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks;
import org.apache.lucene.util.QuickPatchThreadsFilter;
import org.apache.lucene.util.TestUtil;
+import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.embedded.JettyConfig;
-import org.apache.solr.client.solrj.impl.HttpClientConfigurer;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.ConcurrentUpdateSolrClient;
import org.apache.solr.client.solrj.impl.HttpClientUtil;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder;
import org.apache.solr.client.solrj.impl.LBHttpSolrClient;
-import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.util.ClientUtils;
import org.apache.solr.cloud.IpTables;
import org.apache.solr.common.SolrDocument;
@@ -280,7 +279,7 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
System.clearProperty("urlScheme");
System.clearProperty("solr.peerSync.useRangeVersions");
- HttpClientUtil.setConfigurer(new HttpClientConfigurer());
+ HttpClientUtil.resetConfigurers();
// clean up static
sslConfig = null;