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.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.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 NamedList request(SolrRequest request, String coreName) 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 NamedList request(SolrRequest request, String coreName) 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 NamedList request(SolrRequest request, String coreName) 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 NamedList request(SolrRequest request, String coreName) 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 resolved = - (NamedList) new JavaBinCodec(resolver).unmarshal(in); - return resolved; - } - } - } catch (Exception ex) { - throw new RuntimeException(ex); - } - } - - // Now write it out - NamedList 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 @@ ByteArrayInputStream toInputStream() { } } - private Set 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 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 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 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 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 @@ private Set getContentStreams(SolrRequest request) throws IOEx final byte[] buf = baos.toByteArray(); if (buf.length > 0) { - return Collections.singleton( + return List.of( new ContentStreamBase() { @Override @@ -323,7 +357,7 @@ public String getContentType() { }); } - 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.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 @@ default CoreContainer getCoreContainer() { 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 @@ protected void logAndFlushAdminRequest(SolrQueryResponse solrResp) throws IOExce * 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 SolrQueryRequest buildRequestFrom( private SolrQueryRequest buildRequestFrom( SolrCore core, SolrParams params, - Collection streams, + Collection 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 HttpSolrCall getHttpSolrCall() { 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.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 static void beforeClass() throws Exception { 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 final T process(SolrClient client, String collection) 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.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 {