SOLR-17316: make SolrJ response objects work with non-binary ResponseParsers

changelog/unreleased/SOLR-17316-response-parsers.yml

@@ -0,0 +1,12 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+
+title: >
+  SolrJ's QueryResponse and other response objects now work when the client is configured with a
+  non-binary response parser (such as the JSON parser); previously their accessors could throw a
+  ClassCastException.
+type: fixed
+authors:
+  - name: Serhiy Bzhezytskyy
+links:
+  - name: SOLR-17316
+    url: https://issues.apache.org/jira/browse/SOLR-17316

solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java

@@ -246,6 +246,7 @@ private static SolrParams getParams(SolrRequest<?> request) {
       responseParser = new JavaBinResponseParser();
     }
     var addParams = SolrParams.of(CommonParams.WT, responseParser.getWriterType());
+    addParams = SolrParams.wrapDefaults(addParams, responseParser.getAdditionalRequestParams());
     return SolrParams.wrapDefaults(addParams, params);
   }
 
@@ -302,7 +303,7 @@ public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOExcepti
     }
 
     // note: don't bother using the Reader variant; it often throws UnsupportedOperationException
-    return responseParser.processResponse(byteBuffer.toInputStream(), null);
+    return responseParser.processCanonicalResponse(byteBuffer.toInputStream(), null);
   }
 
   /** A list of streams, non-null. */

solr/core/src/test/org/apache/solr/client/solrj/embedded/TestEmbeddedSolrServerResponseParser.java

@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.solrj.embedded;
+
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.request.SolrQuery;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.client.solrj.response.json.JsonMapResponseParser;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.util.EmbeddedSolrServerTestRule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/**
+ * EmbeddedSolrServer reads the response with the configured parser just as the HTTP clients do, so
+ * a non-binary parser has to work here too.
+ */
+public class TestEmbeddedSolrServerResponseParser extends SolrTestCase {
+
+  @ClassRule
+  public static final EmbeddedSolrServerTestRule solrTestRule = new EmbeddedSolrServerTestRule();
+
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    solrTestRule.startSolr(SolrTestCaseJ4.TEST_HOME());
+    SolrTestCaseJ4.newRandomConfig();
+    solrTestRule
+        .newCollection()
+        .withConfigSet(SolrTestCaseJ4.TEST_COLL1_CONF())
+        .withSchemaFile("schema-nest.xml")
+        .create();
+
+    SolrInputDocument doc = new SolrInputDocument();
+    doc.addField("id", "1");
+    doc.addField("name_s", "embedded json");
+    SolrClient client = solrTestRule.getSolrClient();
+    client.add(doc);
+    client.commit();
+  }
+
+  @Test
+  public void testQueryResponseWithJsonParser() throws Exception {
+    SolrQuery q = new SolrQuery("id:1");
+    q.addFacetField("name_s");
+    QueryRequest req = new QueryRequest(q);
+    req.setResponseParser(new JsonMapResponseParser());
+
+    QueryResponse rsp = req.process(solrTestRule.getSolrClient());
+
+    // Header getters cast the values they read, and the JSON writer emits Long where javabin emits
+    // Integer.
+    assertEquals(0, rsp.getStatus());
+    assertNotNull(rsp.getResponseHeader());
+
+    // A facet section is a NamedList; under the default json.nl=flat it arrives as an array of
+    // alternating names and values, which cannot be recovered.
+    assertNotNull("facet_counts must be readable", rsp.getFacetField("name_s"));
+
+    // The documents section has to arrive as a SolrDocumentList for getResults() to work at all.
+    assertEquals(1, rsp.getResults().getNumFound());
+    assertEquals("1", rsp.getResults().get(0).getFirstValue("id"));
+  }
+
+  /**
+   * A named nested document has to come back as a document rather than a plain map, matching what
+   * the binary and XML parsers produce for the same response.
+   */
+  @Test
+  public void testNamedNestedDocumentsWithJsonParser() throws Exception {
+    SolrClient client = solrTestRule.getSolrClient();
+
+    SolrInputDocument child = new SolrInputDocument();
+    child.addField("id", "20");
+    child.addField("name_s", "a comment");
+
+    SolrInputDocument parent = new SolrInputDocument();
+    parent.addField("id", "10");
+    parent.addField("name_s", "a parent");
+    parent.addField("comment", child);
+
+    client.add(parent);
+    client.commit();
+
+    SolrQuery q = new SolrQuery("id:10");
+    q.setFields("*", "[child]");
+    QueryRequest req = new QueryRequest(q);
+    req.setResponseParser(new JsonMapResponseParser());
+
+    QueryResponse rsp = req.process(client);
+
+    SolrDocument doc = rsp.getResults().get(0);
+    Object comment = doc.getFieldValue("comment");
+    assertNotNull("the named child must be present", comment);
+    assertTrue(
+        "a named child must be a SolrDocument, not " + comment.getClass().getName(),
+        comment instanceof SolrDocument);
+    assertEquals("a comment", ((SolrDocument) comment).getFirstValue("name_s"));
+  }
+}

solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java

@@ -48,6 +48,7 @@
 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.SolrParams;
 import org.apache.solr.common.util.ContentStream;
 import org.apache.solr.common.util.NamedList;
 import org.slf4j.Logger;
@@ -135,10 +136,11 @@ public RequestWriter getRequestWriter() {
 
   protected ModifiableSolrParams initializeSolrParams(
       SolrRequest<?> solrRequest, ResponseParser parserToUse) {
-    // The parser 'wt=' param is used instead of the original params
-    ModifiableSolrParams wparams = new ModifiableSolrParams(solrRequest.getParams());
-    wparams.set(CommonParams.WT, parserToUse.getWriterType());
-    return wparams;
+
+    var addParams = SolrParams.of(CommonParams.WT, parserToUse.getWriterType());
+    addParams = SolrParams.wrapDefaults(addParams, parserToUse.getAdditionalRequestParams());
+
+    return new ModifiableSolrParams(SolrParams.wrapDefaults(addParams, solrRequest.getParams()));
   }
 
   protected boolean isMultipart(Collection<ContentStream> streams) {
@@ -228,7 +230,7 @@ protected NamedList<Object> processErrorsAndResponse(
 
       NamedList<Object> rsp;
       try {
-        rsp = processor.processResponse(is, encoding);
+        rsp = processor.processCanonicalResponse(is, encoding);
       } catch (Exception e) {
         throw new RemoteSolrException(urlExceptionMessage, httpStatus, e.getMessage(), e);
       }

solr/solrj/src/java/org/apache/solr/client/solrj/response/AnalysisResponseBase.java

@@ -117,9 +117,9 @@ protected TokenInfo buildTokenInfo(NamedList<?> tokenNL) {
     String text = (String) tokenNL.get("text");
     String rawText = (String) tokenNL.get("rawText");
     String type = (String) tokenNL.get("type");
-    int start = (Integer) tokenNL.get("start");
-    int end = (Integer) tokenNL.get("end");
-    int position = (Integer) tokenNL.get("position");
+    int start = ((Number) tokenNL.get("start")).intValue();
+    int end = ((Number) tokenNL.get("end")).intValue();
+    int position = ((Number) tokenNL.get("position")).intValue();
     Boolean match = (Boolean) tokenNL.get("match");
     return new TokenInfo(
         text, rawText, type, start, end, position, (match == null ? false : match));

solr/solrj/src/java/org/apache/solr/client/solrj/response/LukeResponse.java

@@ -139,7 +139,7 @@ public void read(NamedList<Object> nl) {
         } else if ("docs".equals(entry.getKey())) {
           docs = ((Number) entry.getValue()).longValue();
         } else if ("distinct".equals(entry.getKey())) {
-          distinct = (Integer) entry.getValue();
+          distinct = ((Number) entry.getValue()).intValue();
         } else if ("cacheableFaceting".equals(entry.getKey())) {
           cacheableFaceting = (Boolean) entry.getValue();
         } else if ("topTerms".equals(entry.getKey())) {
@@ -290,7 +290,8 @@ public Long getNumDocs() {
 
   public Integer getMaxDoc() {
     if (indexInfo == null) return null;
-    return (Integer) indexInfo.get("maxDoc");
+    Object v = indexInfo.get("maxDoc");
+    return v == null ? null : ((Number) v).intValue();
   }
 
   public Long getDeletedDocs() {
@@ -299,7 +300,8 @@ public Long getDeletedDocs() {
 
   public Integer getNumTerms() {
     if (indexInfo == null) return null;
-    return (Integer) indexInfo.get("numTerms");
+    Object v = indexInfo.get("numTerms");
+    return v == null ? null : ((Number) v).intValue();
   }
 
   public Map<String, FieldTypeInfo> getFieldTypeInfo() {

solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java

@@ -248,11 +248,11 @@ private void extractGroupedInfo(NamedList<Object> info) {
         }
 
         if (oGroups != null) {
-          Integer iMatches = (Integer) oMatches;
+          int iMatches = ((Number) oMatches).intValue();
           ArrayList<Object> groupsArr = (ArrayList<Object>) oGroups;
           GroupCommand groupedCommand;
           if (oNGroups != null) {
-            Integer iNGroups = (Integer) oNGroups;
+            int iNGroups = ((Number) oNGroups).intValue();
             groupedCommand = new GroupCommand(fieldName, iMatches, iNGroups);
           } else {
             groupedCommand = new GroupCommand(fieldName, iMatches);
@@ -269,10 +269,10 @@ private void extractGroupedInfo(NamedList<Object> info) {
 
           _groupResponse.add(groupedCommand);
         } else if (queryCommand != null) {
-          Integer iMatches = (Integer) oMatches;
+          int iMatches = ((Number) oMatches).intValue();
           GroupCommand groupCommand;
           if (oNGroups != null) {
-            Integer iNGroups = (Integer) oNGroups;
+            int iNGroups = ((Number) oNGroups).intValue();
             groupCommand = new GroupCommand(fieldName, iMatches, iNGroups);
           } else {
             groupCommand = new GroupCommand(fieldName, iMatches);
@@ -302,10 +302,10 @@ private void extractHighlightingInfo(NamedList<Object> info) {
   private void extractFacetInfo(NamedList<Object> info) {
     // Parse the queries
     _facetQuery = new LinkedHashMap<>();
-    NamedList<Integer> fq = (NamedList<Integer>) info.get("facet_queries");
+    NamedList<Number> fq = (NamedList<Number>) info.get("facet_queries");
     if (fq != null) {
-      for (Map.Entry<String, Integer> entry : fq) {
-        _facetQuery.put(entry.getKey(), entry.getValue());
+      for (Map.Entry<String, Number> entry : fq) {
+        _facetQuery.put(entry.getKey(), entry.getValue().intValue());
       }
     }
 
@@ -354,7 +354,9 @@ private void extractFacetInfo(NamedList<Object> info) {
         List<IntervalFacet.Count> counts =
             new ArrayList<IntervalFacet.Count>(intervalField.getValue().size());
         for (Map.Entry<String, Object> interval : intervalField.getValue()) {
-          counts.add(new IntervalFacet.Count(interval.getKey(), (Integer) interval.getValue()));
+          counts.add(
+              new IntervalFacet.Count(
+                  interval.getKey(), ((Number) interval.getValue()).intValue()));
         }
         _intervalFacets.add(new IntervalFacet(field, counts));
       }
@@ -401,9 +403,9 @@ private List<RangeFacet> extractRangeFacets(NamedList<NamedList<Object>> rf) {
             new RangeFacet.Currency(facet.getKey(), start, end, gap, before, after, between);
       }
 
-      NamedList<Integer> counts = (NamedList<Integer>) values.get("counts");
-      for (Map.Entry<String, Integer> entry : counts) {
-        rangeFacet.addCount(entry.getKey(), entry.getValue());
+      NamedList<Number> counts = (NamedList<Number>) values.get("counts");
+      for (Map.Entry<String, Number> entry : counts) {
+        rangeFacet.addCount(entry.getKey(), entry.getValue().intValue());
       }
 
       facetRanges.add(rangeFacet);
@@ -433,7 +435,7 @@ protected List<PivotField> readPivots(List<NamedList> list) {
         switch (key) {
           case "field" -> field = (String) val;
           case "value" -> value = val;
-          case "count" -> count = ((Integer) val).intValue();
+          case "count" -> count = ((Number) val).intValue();
           case "pivot" -> {
             assert null != val : "Server sent back 'null' for sub pivots?";
             assert val instanceof List : "Server sent non-List for sub pivots?";
@@ -447,10 +449,10 @@ protected List<PivotField> readPivots(List<NamedList> list) {
           case "queries" -> {
             // Parse the queries
             queryCounts = new LinkedHashMap<>();
-            NamedList<Integer> fq = (NamedList<Integer>) val;
+            NamedList<Number> fq = (NamedList<Number>) val;
             if (fq != null) {
-              for (Map.Entry<String, Integer> e : fq) {
-                queryCounts.put(e.getKey(), e.getValue());
+              for (Map.Entry<String, Number> e : fq) {
+                queryCounts.put(e.getKey(), e.getValue().intValue());
               }
             }
           }