This is an automated email from the ASF dual-hosted git repository.
dsmiley 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 c5c538a9e02 SOLR-17390: EmbeddedSolrServer now considers the
ResponseParser (#2774)
c5c538a9e02 is described below
commit c5c538a9e025bda77ad591ee82beaa6a6732c408
Author: David Smiley <[email protected]>
AuthorDate: Mon Nov 4 17:45:07 2024 -0500
SOLR-17390: EmbeddedSolrServer now considers the ResponseParser (#2774)
And
* Moved HttpSolrCall.getResponseWriter to SolrQueryRequest
* Subtle improvements to make ContentStream work when they might not have
---
solr/CHANGES.txt | 2 +
.../client/solrj/embedded/EmbeddedSolrServer.java | 162 +++++++++++++--------
.../org/apache/solr/request/SolrQueryRequest.java | 14 ++
.../java/org/apache/solr/servlet/HttpSolrCall.java | 8 +-
.../apache/solr/servlet/SolrRequestParsers.java | 11 +-
.../apache/solr/response/TestRawTransformer.java | 7 +-
.../org/apache/solr/client/solrj/SolrRequest.java | 3 +-
.../solr/client/solrj/impl/NoOpResponseParser.java | 3 +-
8 files changed, 133 insertions(+), 77 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 35070264402..e8008ded0d1 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -154,6 +154,8 @@ Improvements
* SOLR-17528: Introduce -y short option to bin/solr start --no-prompt option.
Aligns with bin/solr package tool. (Eric Pugh)
+* SOLR-17390: EmbeddedSolrServer now considers the ResponseParser (David
Smiley)
+
Optimizations
---------------------
* SOLR-14985: Solrj CloudSolrClient with Solr URLs had serious performance
regressions (since the
diff --git
a/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
b/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
index 49a887c9772..1da77d8db82 100644
---
a/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
+++
b/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
@@ -23,25 +23,28 @@ import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Path;
+import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
import java.util.Properties;
-import java.util.Set;
import java.util.function.Supplier;
import org.apache.lucene.search.TotalHits.Relation;
+import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.StreamingResponseCallback;
import org.apache.solr.client.solrj.impl.BinaryRequestWriter;
+import org.apache.solr.client.solrj.impl.BinaryResponseParser;
+import org.apache.solr.client.solrj.impl.InputStreamResponseParser;
import org.apache.solr.client.solrj.request.ContentStreamUpdateRequest;
import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CommonParams;
-import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.MapSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;
import org.apache.solr.common.util.ContentStreamBase;
@@ -55,6 +58,7 @@ import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.response.BinaryResponseWriter;
+import org.apache.solr.response.QueryResponseWriterUtil;
import org.apache.solr.response.ResultContext;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.servlet.SolrRequestParsers;
@@ -165,13 +169,13 @@ public class EmbeddedSolrServer extends SolrClient {
try {
SolrQueryRequest req =
_parser.buildRequestFrom(
- null, request.getParams(), getContentStreams(request),
request.getUserPrincipal());
+ null, getParams(request), getContentStreams(request),
request.getUserPrincipal());
req.getContext().put("httpMethod", request.getMethod().name());
req.getContext().put(PATH, path);
SolrQueryResponse resp = new SolrQueryResponse();
handler.handleRequest(req, resp);
checkForExceptions(resp);
- return BinaryResponseWriter.getParsedResponse(req, resp);
+ return writeResponse(request, req, resp);
} catch (IOException | SolrException iox) {
throw iox;
} catch (Exception ex) {
@@ -196,10 +200,7 @@ public class EmbeddedSolrServer extends SolrClient {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "No such
core: " + coreName);
}
- SolrParams params = request.getParams();
- if (params == null) {
- params = new ModifiableSolrParams();
- }
+ SolrParams params = getParams(request);
// Extract the handler from the path or params
handler = core.getRequestHandler(path);
@@ -217,8 +218,10 @@ public class EmbeddedSolrServer extends SolrClient {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "unknown
handler: " + path);
}
req =
- _parser.buildRequestFrom(
- core, params, getContentStreams(request),
request.getUserPrincipal());
+ core.getSolrConfig()
+ .getRequestParsers()
+ .buildRequestFrom(
+ core, params, getContentStreams(request),
request.getUserPrincipal());
req.getContext().put(PATH, path);
req.getContext().put("httpMethod", request.getMethod().name());
SolrQueryResponse rsp = new SolrQueryResponse();
@@ -226,53 +229,7 @@ public class EmbeddedSolrServer extends SolrClient {
core.execute(handler, req, rsp);
checkForExceptions(rsp);
-
- // Check if this should stream results
- if (request.getStreamingResponseCallback() != null) {
- try {
- final StreamingResponseCallback callback =
request.getStreamingResponseCallback();
- BinaryResponseWriter.Resolver resolver =
- new BinaryResponseWriter.Resolver(req, rsp.getReturnFields()) {
- @Override
- public void writeResults(ResultContext ctx, JavaBinCodec
codec) throws IOException {
- // write an empty list...
- SolrDocumentList docs = new SolrDocumentList();
- docs.setNumFound(ctx.getDocList().matches());
- docs.setNumFoundExact(ctx.getDocList().hitCountRelation() ==
Relation.EQUAL_TO);
- docs.setStart(ctx.getDocList().offset());
- docs.setMaxScore(ctx.getDocList().maxScore());
- codec.writeSolrDocumentList(docs);
-
- // This will transform
- writeResultsBody(ctx, codec);
- }
- };
-
- try (var out =
- new ByteArrayOutputStream() {
- ByteArrayInputStream toInputStream() {
- return new ByteArrayInputStream(buf, 0, count);
- }
- }) {
- createJavaBinCodec(callback, resolver)
- .setWritableDocFields(resolver)
- .marshal(rsp.getValues(), out);
-
- try (ByteArrayInputStream in = out.toInputStream()) {
- @SuppressWarnings({"unchecked"})
- NamedList<Object> resolved =
- (NamedList<Object>) new JavaBinCodec(resolver).unmarshal(in);
- return resolved;
- }
- }
- } catch (Exception ex) {
- throw new RuntimeException(ex);
- }
- }
-
- // Now write it out
- NamedList<Object> normalized =
BinaryResponseWriter.getParsedResponse(req, rsp);
- return normalized;
+ return writeResponse(request, req, rsp);
} catch (IOException | SolrException iox) {
throw iox;
} catch (Exception ex) {
@@ -285,12 +242,89 @@ public class EmbeddedSolrServer extends SolrClient {
}
}
- private Set<ContentStream> getContentStreams(SolrRequest<?> request) throws
IOException {
- if (request.getMethod() == SolrRequest.METHOD.GET) return null;
+ private static SolrParams getParams(SolrRequest<?> request) {
+ var params = request.getParams();
+ var responseParser = request.getResponseParser();
+ if (responseParser == null) {
+ responseParser = new BinaryResponseParser();
+ }
+ var addParams =
+ new MapSolrParams(
+ Map.of(
+ CommonParams.WT,
+ responseParser.getWriterType(),
+ CommonParams.VERSION,
+ responseParser.getVersion()));
+ return SolrParams.wrapDefaults(addParams, params);
+ }
+
+ private NamedList<Object> writeResponse(
+ SolrRequest<?> request, SolrQueryRequest req, SolrQueryResponse rsp)
throws IOException {
+ ResponseParser responseParser = request.getResponseParser();
+ if (responseParser == null) {
+ responseParser = new BinaryResponseParser();
+ }
+ StreamingResponseCallback callback =
request.getStreamingResponseCallback();
+ // TODO refactor callback to be a special responseParser that we check for
+ // TODO if responseParser is a special/internal NamedList ResponseParser,
just return NL
+
+ var byteBuffer =
+ new ByteArrayOutputStream() {
+ ByteArrayInputStream toInputStream() {
+ return new ByteArrayInputStream(buf, 0, count);
+ }
+ };
+
+ if (callback == null) {
+ QueryResponseWriterUtil.writeQueryResponse(
+ byteBuffer, req.getResponseWriter(), req, rsp, null);
+ } else {
+ // mostly stream results to the callback; rest goes into the byteBuffer
+ if (!(responseParser instanceof BinaryResponseParser))
+ throw new IllegalArgumentException(
+ "Only javabin is supported when using a streaming response
callback");
+ var resolver =
+ new BinaryResponseWriter.Resolver(req, rsp.getReturnFields()) {
+ @Override
+ public void writeResults(ResultContext ctx, JavaBinCodec codec)
throws IOException {
+ // write an empty list...
+ SolrDocumentList docs = new SolrDocumentList();
+ docs.setNumFound(ctx.getDocList().matches());
+ docs.setNumFoundExact(ctx.getDocList().hitCountRelation() ==
Relation.EQUAL_TO);
+ docs.setStart(ctx.getDocList().offset());
+ docs.setMaxScore(ctx.getDocList().maxScore());
+ codec.writeSolrDocumentList(docs);
+
+ // This will transform
+ writeResultsBody(ctx, codec);
+ }
+ };
+
+ // invoke callbacks, and writes the rest to byteBuffer
+ try (var javaBinCodec = createJavaBinCodec(callback, resolver)) {
+ javaBinCodec.setWritableDocFields(resolver).marshal(rsp.getValues(),
byteBuffer);
+ }
+ }
+
+ if (responseParser instanceof InputStreamResponseParser) {
+ // SPECIAL CASE
+ NamedList<Object> namedList = new NamedList<>();
+ namedList.add("stream", byteBuffer.toInputStream());
+ namedList.add("responseStatus", 200); // always by this point
+ return namedList;
+ }
+
+ // note: don't bother using the Reader variant; it often throws
UnsupportedOperationException
+ return responseParser.processResponse(byteBuffer.toInputStream(), null);
+ }
+
+ /** A list of streams, non-null. */
+ private List<ContentStream> getContentStreams(SolrRequest<?> request) throws
IOException {
+ if (request.getMethod() == SolrRequest.METHOD.GET) return List.of();
if (request instanceof ContentStreamUpdateRequest) {
final ContentStreamUpdateRequest csur = (ContentStreamUpdateRequest)
request;
final Collection<ContentStream> cs = csur.getContentStreams();
- if (cs != null) return new HashSet<>(cs);
+ if (cs != null) return new ArrayList<>(cs);
}
final RequestWriter.ContentWriter contentWriter =
request.getContentWriter(null);
@@ -308,7 +342,7 @@ public class EmbeddedSolrServer extends SolrClient {
final byte[] buf = baos.toByteArray();
if (buf.length > 0) {
- return Collections.singleton(
+ return List.of(
new ContentStreamBase() {
@Override
@@ -323,7 +357,7 @@ public class EmbeddedSolrServer extends SolrClient {
});
}
- return null;
+ return List.of();
}
private JavaBinCodec createJavaBinCodec(
diff --git a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
index ab020ff2509..4b2a4d543d4 100644
--- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
+++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
@@ -29,6 +29,7 @@ import org.apache.solr.common.util.ContentStream;
import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.QueryResponseWriter;
import org.apache.solr.schema.IndexSchema;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.servlet.HttpSolrCall;
@@ -188,4 +189,17 @@ public interface SolrQueryRequest extends AutoCloseable {
default CloudDescriptor getCloudDescriptor() {
return getCore().getCoreDescriptor().getCloudDescriptor();
}
+
+ /** The writer to use for this request, considering {@link CommonParams#WT}.
Never null. */
+ default QueryResponseWriter getResponseWriter() {
+ // it's weird this method is here instead of SolrQueryResponse, but it's
practical/convenient
+ SolrCore core = getCore();
+ String wt = getParams().get(CommonParams.WT);
+ if (core != null) {
+ return core.getQueryResponseWriter(wt);
+ } else {
+ return SolrCore.DEFAULT_RESPONSE_WRITERS.getOrDefault(
+ wt, SolrCore.DEFAULT_RESPONSE_WRITERS.get("standard"));
+ }
+ }
}
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index 9a8fcd400b6..cc645476aef 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -934,13 +934,7 @@ public class HttpSolrCall {
* returns the default query response writer Note: This method must not
return null
*/
protected QueryResponseWriter getResponseWriter() {
- String wt = solrReq.getParams().get(CommonParams.WT);
- if (core != null) {
- return core.getQueryResponseWriter(wt);
- } else {
- return SolrCore.DEFAULT_RESPONSE_WRITERS.getOrDefault(
- wt, SolrCore.DEFAULT_RESPONSE_WRITERS.get("standard"));
- }
+ return solrReq.getResponseWriter();
}
protected void handleAdmin(SolrQueryResponse solrResp) {
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
index 38a45c71a91..ee459c29231 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
@@ -197,11 +197,18 @@ public class SolrRequestParsers {
private SolrQueryRequest buildRequestFrom(
SolrCore core,
SolrParams params,
- Collection<ContentStream> streams,
+ Collection<ContentStream> streams, // might be added to but caller
shouldn't depend on it
RTimerTree requestTimer,
final HttpServletRequest req,
final Principal principal)
throws Exception {
+ // ensure streams is non-null and mutable so we can easily add to it
+ if (streams == null) {
+ streams = new ArrayList<>();
+ } else if (!(streams instanceof ArrayList)) {
+ streams = new ArrayList<>(streams);
+ }
+
// The content type will be applied to all streaming content
String contentType = params.get(CommonParams.STREAM_CONTENTTYPE);
@@ -293,7 +300,7 @@ public class SolrRequestParsers {
return httpSolrCall;
}
};
- if (streams != null && streams.size() > 0) {
+ if (!streams.isEmpty()) {
q.setContentStreams(streams);
}
return q;
diff --git
a/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
b/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
index b352f177e42..77de71eca38 100644
--- a/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
+++ b/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
@@ -26,6 +26,7 @@ import java.util.Properties;
import java.util.regex.Pattern;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.NoOpResponseParser;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
@@ -60,7 +61,11 @@ public class TestRawTransformer extends SolrCloudTestCase {
if (random().nextBoolean()) {
initStandalone();
JSR.start();
- CLIENT = JSR.newClient();
+ if (random().nextBoolean()) {
+ CLIENT = JSR.newClient();
+ } else {
+ CLIENT = new EmbeddedSolrServer(JSR.getCoreContainer(), null);
+ }
} else {
initCloud();
CLIENT = JSR.newClient();
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 a615d478b98..58a1c239683 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
@@ -250,7 +250,8 @@ public abstract class SolrRequest<T extends SolrResponse>
implements Serializabl
throws SolrServerException, IOException {
long startNanos = System.nanoTime();
T res = createResponse(client);
- res.setResponse(client.request(this, collection));
+ var namedList = client.request(this, collection);
+ res.setResponse(namedList);
long endNanos = System.nanoTime();
res.setElapsedTime(TimeUnit.NANOSECONDS.toMillis(endNanos - startNanos));
return res;
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/NoOpResponseParser.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/NoOpResponseParser.java
index 919b6b873f1..92101918db5 100644
---
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/NoOpResponseParser.java
+++
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/NoOpResponseParser.java
@@ -26,8 +26,7 @@ import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
/**
- * Simply puts the entire response into an entry in a NamedList. This parser
isn't parse response
- * into a QueryResponse.
+ * A special parser that puts the entire response into a string "response"
field in the NamedList.
*/
public class NoOpResponseParser extends ResponseParser {
