From d1499bffa368698082c4f0da617d97082dd2cdf1 Mon Sep 17 00:00:00 2001 From: Gregory Chanan Date: Fri, 26 Sep 2014 22:04:14 +0000 Subject: [PATCH] SOLR-6565: SolrRequest support for query params git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1627898 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 2 + .../apache/solr/client/solrj/SolrRequest.java | 15 +++- .../client/solrj/impl/HttpSolrServer.java | 42 +++++---- .../solrj/impl/BasicHttpSolrServerTest.java | 89 ++++++++++++++++++- 4 files changed, 131 insertions(+), 17 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 0de774ce53c..9444bb5be04 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -143,6 +143,8 @@ New Features * SOLR-5986: Don't allow runaway queries from harming Solr cluster health or search performance (Anshum Gupta, Steve Rowe, Robert Muir) +* SOLR-6565: SolrRequest support for query params (Gregory Chanan) + Bug Fixes ---------------------- diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java index 0aecf281eab..05346ca5a09 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java @@ -20,6 +20,7 @@ package org.apache.solr.client.solrj; import java.io.IOException; import java.io.Serializable; import java.util.Collection; +import java.util.Set; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.ContentStream; @@ -42,6 +43,7 @@ public abstract class SolrRequest implements Serializable private ResponseParser responseParser; private StreamingResponseCallback callback; + private Set queryParams; //--------------------------------------------------------- //--------------------------------------------------------- @@ -93,7 +95,18 @@ public abstract class SolrRequest implements Serializable public void setStreamingResponseCallback(StreamingResponseCallback callback) { this.callback = callback; } - + + /** + * Parameter keys that are sent via the query string + */ + public Set getQueryParams() { + return this.queryParams; + } + + public void setQueryParams(Set queryParams) { + this.queryParams = queryParams; + } + public abstract SolrParams getParams(); public abstract Collection getContentStreams() throws IOException; public abstract SolrResponse process( SolrServer server ) throws SolrServerException, IOException; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrServer.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrServer.java index 85bd1d23c18..a2d741175c7 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrServer.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrServer.java @@ -178,8 +178,11 @@ public class HttpSolrServer extends SolrServer { } /** - * Expert Method. + * 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. + * @see org.apache.solr.client.solrj.SolrRequest#getQueryParams */ public void setQueryParams(Set queryParams) { this.queryParams = queryParams; @@ -254,7 +257,24 @@ public class HttpSolrServer extends SolrServer { mrr.httpUriRequest = method; return mrr; } - + + protected ModifiableSolrParams calculateQueryParams(Set queryParamNames, + ModifiableSolrParams wparams) { + ModifiableSolrParams queryModParams = new ModifiableSolrParams(); + if (queryParamNames != null) { + for (String param : queryParamNames) { + String[] value = wparams.getParams(param) ; + if (value != null) { + for (String v : value) { + queryModParams.add(param, v); + } + wparams.remove(param); + } + } + } + return queryModParams; + } + protected HttpRequestBase createMethod(final SolrRequest request) throws IOException, SolrServerException { HttpRequestBase method = null; InputStream is = null; @@ -309,19 +329,11 @@ public class HttpSolrServer extends SolrServer { } boolean isMultipart = ((this.useMultiPartPost && SolrRequest.METHOD.POST == request.getMethod()) || ( streams != null && streams.size() > 1 )) && !hasNullStreamName; - - // only send this list of params as query string params - ModifiableSolrParams queryParams = new ModifiableSolrParams(); - for (String param : this.queryParams) { - String[] value = wparams.getParams(param) ; - if (value != null) { - for (String v : value) { - queryParams.add(param, v); - } - wparams.remove(param); - } - } - + + // send server list and request list as query string params + ModifiableSolrParams queryParams = calculateQueryParams(this.queryParams, wparams); + queryParams.add(calculateQueryParams(request.getQueryParams(), wparams)); + LinkedList postOrPutParams = new LinkedList<>(); if (streams == null || isMultipart) { String fullQueryUrl = url + ClientUtils.toQueryString( queryParams, false ); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrServerTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrServerTest.java index 52126941ced..017c94ab967 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrServerTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrServerTest.java @@ -23,7 +23,10 @@ import java.net.MalformedURLException; import java.net.Socket; import java.util.Enumeration; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -37,6 +40,7 @@ import org.apache.http.client.HttpClient; import org.apache.http.client.methods.HttpGet; import org.apache.solr.SolrJettyTestBase; import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrRequest.METHOD; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.QueryRequest; @@ -46,6 +50,7 @@ import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.params.CommonParams; +import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.util.ExternalPaths; import org.apache.solr.util.SSLTestConfig; @@ -78,12 +83,14 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase { headers = null; parameters = null; errorCode = null; + queryString = null; } public static Integer errorCode = null; public static String lastMethod = null; public static HashMap headers = null; public static Map parameters = null; + public static String queryString = null; public static void setErrorCode(Integer code) { errorCode = code; @@ -110,6 +117,10 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase { parameters = req.getParameterMap(); } + private void setQueryString(HttpServletRequest req) { + queryString = req.getQueryString(); + } + @Override protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { @@ -127,6 +138,7 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase { private void recordRequest(HttpServletRequest req, HttpServletResponse resp) { setHeaders(req); setParameters(req); + setQueryString(req); if (null != errorCode) { try { resp.sendError(errorCode); @@ -553,5 +565,80 @@ public class BasicHttpSolrServerTest extends SolrJettyTestBase { } throw new RuntimeException("Could not find unused TCP port."); } - + + private Set setOf(String... keys) { + Set set = new TreeSet(); + if (keys != null) { + for (String k : keys) { + set.add(k); + } + } + return set; + } + + private void setReqParamsOf(UpdateRequest req, String... keys) { + if (keys != null) { + for (String k : keys) { + req.setParam(k, k+"Value"); + } + } + } + + private void verifyServletState(HttpSolrServer server, SolrRequest request) { + // check query String + Iterator paramNames = request.getParams().getParameterNamesIterator(); + while (paramNames.hasNext()) { + String name = paramNames.next(); + String [] values = request.getParams().getParams(name); + if (values != null) { + for (String value : values) { + boolean shouldBeInQueryString = server.getQueryParams().contains(name) + || (request.getQueryParams() != null && request.getQueryParams().contains(name)); + assertEquals(shouldBeInQueryString, DebugServlet.queryString.contains(name + "=" + value)); + // in either case, it should be in the parameters + assertNotNull(DebugServlet.parameters.get(name)); + assertEquals(1, DebugServlet.parameters.get(name).length); + assertEquals(value, DebugServlet.parameters.get(name)[0]); + } + } + } + } + + @Test + public void testQueryString() throws Exception { + HttpSolrServer server = new HttpSolrServer(jetty.getBaseUrl().toString() + + "/debug/foo"); + + // test without request query params + DebugServlet.clear(); + server.setQueryParams(setOf("serverOnly")); + UpdateRequest req = new UpdateRequest(); + setReqParamsOf(req, "serverOnly", "notServer"); + try { + server.request(req); + } catch (Throwable t) {} + verifyServletState(server, req); + + // test without server query params + DebugServlet.clear(); + server.setQueryParams(setOf()); + req = new UpdateRequest(); + req.setQueryParams(setOf("requestOnly")); + setReqParamsOf(req, "requestOnly", "notRequest"); + try { + server.request(req); + } catch (Throwable t) {} + verifyServletState(server, req); + + // test with both request and server query params + DebugServlet.clear(); + req = new UpdateRequest(); + server.setQueryParams(setOf("serverOnly", "both")); + req.setQueryParams(setOf("requestOnly", "both")); + setReqParamsOf(req, "serverOnly", "requestOnly", "both", "neither"); + try { + server.request(req); + } catch (Throwable t) {} + verifyServletState(server, req); + } }