dsmiley commented on code in PR #2774:
URL: https://github.com/apache/solr/pull/2774#discussion_r1803994463
##########
solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java:
##########
@@ -217,62 +218,18 @@ public NamedList<Object> 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()
Review Comment:
Using a SolrCore's SolrRequestParsers instance instead of a default one on
this class means we work with parsers and configuration specific to the core.
This was previously simply overlooked.
##########
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);
Review Comment:
put on its own line to help debugger step-in. I've meant to do this for
years.
##########
solr/core/src/test/org/apache/solr/response/TestRawTransformer.java:
##########
@@ -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);
Review Comment:
it works; I re-rean the test enough times to see it does, with code
coverage. Not in this PR, I also played with flipping SolrTestCaseHS and some
other ZK endpoint test, which helped improve this PR by uncovering things, but
those changes seemed like too much scope right now.
##########
solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java:
##########
@@ -217,62 +218,18 @@ public NamedList<Object> 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();
SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));
core.execute(handler, req, rsp);
checkForExceptions(rsp);
-
- // Check if this should stream results
- if (request.getStreamingResponseCallback() != null) {
Review Comment:
moved to writeResponse and simplified a little
##########
solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java:
##########
@@ -285,12 +242,89 @@ ByteArrayInputStream toInputStream() {
}
}
- 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
+ var 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);
Review Comment:
Set didn't make sense
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
