From 2c4c5040eecdf7ab63d81e6e5a2e519891684d59 Mon Sep 17 00:00:00 2001 From: Ishan Chattopadhyaya Date: Mon, 19 Dec 2016 21:01:20 +0530 Subject: [PATCH] SOLR-9860: Enable configuring invariantParams via HttpSolrClient.Builder --- solr/CHANGES.txt | 3 + .../impl/DelegationTokenHttpSolrClient.java | 34 ++++++++++- .../client/solrj/impl/HttpSolrClient.java | 56 +++++++++++++++---- .../solrj/impl/BasicHttpSolrClientTest.java | 31 ++++++++++ 4 files changed, 111 insertions(+), 13 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f783934b5a1..35a2bb7b3fa 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -176,6 +176,9 @@ New Features * SOLR-9513: Generic authentication plugins (GenericHadoopAuthPlugin and ConfigurableInternodeAuthHadoopPlugin) that delegate all functionality to Hadoop authentication framework. (Hrishikesh Gadre via Ishan Chattopadhyaya) +* SOLR-9860: Enable configuring invariantParams via HttpSolrClient.Builder (Hrishikesh Gadre, Ishan Chattopadhyaya) + + Optimizations ---------------------- * SOLR-9704: Facet Module / JSON Facet API: Optimize blockChildren facets that have 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 c14e9edf922..ab8175d93d5 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 @@ -33,8 +33,18 @@ import org.apache.solr.common.params.SolrParams; public class DelegationTokenHttpSolrClient extends HttpSolrClient { public final static String DELEGATION_TOKEN_PARAM = "delegation"; - private final String delegationToken; + /** + * This constructor is deprecated in favor of passing delegation token via + * {@linkplain org.apache.solr.client.solrj.impl.HttpSolrClient.Builder#withInvariantParams(ModifiableSolrParams)}. + * + * @param baseURL The base url to communicate with the Solr server + * @param client Http client instance to use for communication + * @param parser Response parser instance to use to decode response from Solr server + * @param allowCompression Should compression be allowed ? + * @param delegationToken The delegation token string. + */ + @Deprecated public DelegationTokenHttpSolrClient(String baseURL, HttpClient client, ResponseParser parser, @@ -44,12 +54,32 @@ public class DelegationTokenHttpSolrClient extends HttpSolrClient { if (delegationToken == null) { throw new IllegalArgumentException("Delegation token cannot be null"); } - this.delegationToken = delegationToken; setQueryParams(new TreeSet<>(Arrays.asList(DELEGATION_TOKEN_PARAM))); invariantParams = new ModifiableSolrParams(); invariantParams.set(DELEGATION_TOKEN_PARAM, delegationToken); } + /** + * This constructor is defined at "protected" scope. Ideally applications should + * use {@linkplain org.apache.solr.client.solrj.impl.HttpSolrClient.Builder} instance + * to configure this Solr client instance. + * + * @param baseURL The base url to communicate with the Solr server + * @param client Http client instance to use for communication + * @param parser Response parser instance to use to decode response from Solr server + * @param allowCompression Should compression be allowed ? + * @param invariantParams The parameters which should be passed with every request. + */ + protected DelegationTokenHttpSolrClient(String baseURL, + HttpClient client, + ResponseParser parser, + boolean allowCompression, + ModifiableSolrParams invariantParams) { + super(baseURL, client, parser, allowCompression, invariantParams); + + setQueryParams(new TreeSet<>(Arrays.asList(DELEGATION_TOKEN_PARAM))); + } + @Override protected HttpRequestBase createMethod(final SolrRequest request, String collection) throws IOException, SolrServerException { SolrParams params = request.getParams(); 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 3e2355cfe70..5b272fcb6f7 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 @@ -29,6 +29,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @@ -212,7 +213,23 @@ public class HttpSolrClient extends SolrClient { this.parser = parser; } - + + /** + * The consturctor. + * + * @param baseURL The base url to communicate with the Solr server + * @param client Http client instance to use for communication + * @param parser Response parser instance to use to decode response from Solr server + * @param allowCompression Should compression be allowed ? + * @param invariantParams The parameters which should be included with every request. + */ + protected HttpSolrClient(String baseURL, HttpClient client, ResponseParser parser, boolean allowCompression, + ModifiableSolrParams invariantParams) { + this(baseURL, client, parser, allowCompression); + + this.invariantParams = invariantParams; + } + public Set getQueryParams() { return queryParams; } @@ -756,7 +773,7 @@ public class HttpSolrClient extends SolrClient { private HttpClient httpClient; private ResponseParser responseParser; private boolean compression; - private String delegationToken; + private ModifiableSolrParams invariantParams = new ModifiableSolrParams(); public Builder() { this.responseParser = new BinaryResponseParser(); @@ -778,7 +795,7 @@ public class HttpSolrClient extends SolrClient { this.baseSolrUrl = baseSolrUrl; this.responseParser = new BinaryResponseParser(); } - + /** * Provides a {@link HttpClient} for the builder to use when creating clients. */ @@ -786,7 +803,7 @@ public class HttpSolrClient extends SolrClient { this.httpClient = httpClient; return this; } - + /** * Provides a {@link ResponseParser} for created clients to use when handling requests. */ @@ -794,7 +811,7 @@ public class HttpSolrClient extends SolrClient { this.responseParser = responseParser; return this; } - + /** * Chooses whether created {@link HttpSolrClient}s use compression by default. */ @@ -807,8 +824,7 @@ public class HttpSolrClient extends SolrClient { * Use a delegation token for authenticating via the KerberosPlugin */ public Builder withKerberosDelegationToken(String delegationToken) { - this.delegationToken = delegationToken; - return this; + return withDelegationToken(delegationToken); } @Deprecated @@ -816,9 +832,26 @@ public class HttpSolrClient extends SolrClient { * @deprecated use {@link withKerberosDelegationToken(String)} instead */ public Builder withDelegationToken(String delegationToken) { - this.delegationToken = delegationToken; + if (this.invariantParams.get(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM) != null) { + throw new IllegalStateException(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM + " is already defined!"); + } + this.invariantParams.add(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM, delegationToken); return this; } + + public Builder withInvariantParams(ModifiableSolrParams params) { + Objects.requireNonNull(params, "params must be non null!"); + + for (String name : params.getParameterNames()) { + if (this.invariantParams.get(name) != null) { + throw new IllegalStateException("parameter " + name + " is redefined."); + } + } + + this.invariantParams.add(params); + return this; + } + /** * Create a {@link HttpSolrClient} based on provided configuration. */ @@ -826,10 +859,11 @@ public class HttpSolrClient extends SolrClient { if (baseSolrUrl == null) { throw new IllegalArgumentException("Cannot create HttpSolrClient without a valid baseSolrUrl!"); } - if (delegationToken == null) { - return new HttpSolrClient(baseSolrUrl, httpClient, responseParser, compression); + + if (this.invariantParams.get(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM) == null) { + return new HttpSolrClient(baseSolrUrl, httpClient, responseParser, compression, invariantParams); } else { - return new DelegationTokenHttpSolrClient(baseSolrUrl, httpClient, responseParser, compression, delegationToken); + return new DelegationTokenHttpSolrClient(baseSolrUrl, httpClient, responseParser, compression, invariantParams); } } } 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 11d2784d9ae..06ae8b81591 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 @@ -54,6 +54,7 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.cookie.BasicClientCookie; import org.apache.http.protocol.HttpContext; import org.apache.solr.SolrJettyTestBase; +import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrRequest.METHOD; @@ -77,6 +78,7 @@ import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + public class BasicHttpSolrClientTest extends SolrJettyTestBase { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -818,4 +820,33 @@ public class BasicHttpSolrClientTest extends SolrJettyTestBase { verifyServletState(client, req); } } + + @Test + public void testInvariantParams() throws IOException { + try(HttpSolrClient createdClient = new HttpSolrClient.Builder() + .withBaseSolrUrl(jetty.getBaseUrl().toString()) + .withInvariantParams(SolrTestCaseJ4.params("param", "value")) + .build()) { + assertEquals("value", createdClient.getInvariantParams().get("param")); + } + + try(HttpSolrClient createdClient = new HttpSolrClient.Builder() + .withBaseSolrUrl(jetty.getBaseUrl().toString()) + .withInvariantParams(SolrTestCaseJ4.params("fq", "fq1", "fq", "fq2")) + .build()) { + assertEquals(2, createdClient.getInvariantParams().getParams("fq").length); + } + + try(HttpSolrClient createdClient = new HttpSolrClient.Builder() + .withBaseSolrUrl(jetty.getBaseUrl().toString()) + .withDelegationToken("mydt") + .withInvariantParams(SolrTestCaseJ4.params(DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM, "mydt")) + .build()) { + fail(); + } catch(Exception ex) { + if (!ex.getMessage().equals("parameter "+ DelegationTokenHttpSolrClient.DELEGATION_TOKEN_PARAM +" is redefined.")) { + throw ex; + } + } + } }