diff --git a/changelog/unreleased/SOLR-17316-response-parsers.yml b/changelog/unreleased/SOLR-17316-response-parsers.yml new file mode 100644 index 000000000000..08cf0145ccf6 --- /dev/null +++ b/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 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 990473ed83ec..8b691f653e6b 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 @@ -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. */ diff --git a/solr/core/src/test/org/apache/solr/client/solrj/embedded/TestEmbeddedSolrServerResponseParser.java b/solr/core/src/test/org/apache/solr/client/solrj/embedded/TestEmbeddedSolrServerResponseParser.java new file mode 100644 index 000000000000..f4adbc0b4827 --- /dev/null +++ b/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")); + } +} diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index fccc4357fcde..a31e4d8666af 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/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 streams) { @@ -228,7 +230,7 @@ protected NamedList processErrorsAndResponse( NamedList rsp; try { - rsp = processor.processResponse(is, encoding); + rsp = processor.processCanonicalResponse(is, encoding); } catch (Exception e) { throw new RemoteSolrException(urlExceptionMessage, httpStatus, e.getMessage(), e); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/AnalysisResponseBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/AnalysisResponseBase.java index f458bdc01c93..5c6f3851826f 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/AnalysisResponseBase.java +++ b/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)); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/LukeResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/LukeResponse.java index f23bb29cffab..bfe75abb4893 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/LukeResponse.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/LukeResponse.java @@ -139,7 +139,7 @@ public void read(NamedList 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 getFieldTypeInfo() { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java index 392d61801227..3e9e472f3597 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/QueryResponse.java @@ -248,11 +248,11 @@ private void extractGroupedInfo(NamedList info) { } if (oGroups != null) { - Integer iMatches = (Integer) oMatches; + int iMatches = ((Number) oMatches).intValue(); ArrayList groupsArr = (ArrayList) 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 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 info) { private void extractFacetInfo(NamedList info) { // Parse the queries _facetQuery = new LinkedHashMap<>(); - NamedList fq = (NamedList) info.get("facet_queries"); + NamedList fq = (NamedList) info.get("facet_queries"); if (fq != null) { - for (Map.Entry entry : fq) { - _facetQuery.put(entry.getKey(), entry.getValue()); + for (Map.Entry entry : fq) { + _facetQuery.put(entry.getKey(), entry.getValue().intValue()); } } @@ -354,7 +354,9 @@ private void extractFacetInfo(NamedList info) { List counts = new ArrayList(intervalField.getValue().size()); for (Map.Entry 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 extractRangeFacets(NamedList> rf) { new RangeFacet.Currency(facet.getKey(), start, end, gap, before, after, between); } - NamedList counts = (NamedList) values.get("counts"); - for (Map.Entry entry : counts) { - rangeFacet.addCount(entry.getKey(), entry.getValue()); + NamedList counts = (NamedList) values.get("counts"); + for (Map.Entry entry : counts) { + rangeFacet.addCount(entry.getKey(), entry.getValue().intValue()); } facetRanges.add(rangeFacet); @@ -433,7 +435,7 @@ protected List readPivots(List 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 readPivots(List list) { case "queries" -> { // Parse the queries queryCounts = new LinkedHashMap<>(); - NamedList fq = (NamedList) val; + NamedList fq = (NamedList) val; if (fq != null) { - for (Map.Entry e : fq) { - queryCounts.put(e.getKey(), e.getValue()); + for (Map.Entry e : fq) { + queryCounts.put(e.getKey(), e.getValue().intValue()); } } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseNormalizer.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseNormalizer.java new file mode 100644 index 000000000000..270c9c0e6a94 --- /dev/null +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseNormalizer.java @@ -0,0 +1,147 @@ +/* + * 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.response; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.params.CommonParams; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; + +/** + * Converts a parsed response into the canonical shape the SolrJ response classes expect (the shape + * the binary and XML parsers produce): nested JSON objects become {@link NamedList}s and a {@code + * {numFound, docs}} object becomes a {@link SolrDocumentList}. + * + *

Only unambiguous, self-describing conversions are performed. It is a no-op for values already + * in canonical form (so binary/XML responses pass through unchanged). It does not attempt to + * interpret the ambiguous flat arrays produced by {@code json.nl=flat}; a typed JSON parser should + * request {@code json.nl=map} for its own reads. + * + *

Public only so that a {@link ResponseParser} in another package can reach it from {@link + * ResponseParser#processCanonicalResponse}; it is not intended for callers. + * + * @lucene.internal + */ +public final class ResponseNormalizer { + + private ResponseNormalizer() {} + + /** Returns a normalized copy of the given response NamedList. */ + public static NamedList normalize(NamedList response) { + if (response == null) { + return null; + } + SimpleOrderedMap out = new SimpleOrderedMap<>(response.size()); + for (Map.Entry e : response) { + out.add(e.getKey(), normalizeValue(e.getValue())); + } + return out; + } + + @SuppressWarnings("unchecked") + private static Object normalizeValue(Object val) { + if (val instanceof SolrDocumentList || val instanceof SolrDocument) { + // Already canonical (binary/XML produce these directly); leave untouched. Must precede the + // List/Map branches since SolrDocumentList is a List and SolrDocument is a Map. + return val; + } else if (val instanceof NamedList in) { + // Already canonical (binary/XML), but its children may still need normalizing. Keep the + // concrete type: a SimpleOrderedMap asserts unique keys, which a general NamedList does not, + // so promoting one to the other would change the contract of the value. + NamedList out = + in instanceof SimpleOrderedMap + ? new SimpleOrderedMap<>(in.size()) + : new NamedList<>(in.size()); + for (Map.Entry e : in) { + out.add(e.getKey(), normalizeValue(e.getValue())); + } + return out; + } else if (val instanceof Map raw) { + Map m = (Map) raw; + if (isDocList(m)) { + return toDocList(m); + } + if (isNestedDoc(m)) { + return toDoc(m); + } + // A JSON object has unique keys by construction, so it maps onto SimpleOrderedMap. + SimpleOrderedMap out = new SimpleOrderedMap<>(m.size()); + for (Map.Entry e : m.entrySet()) { + out.add(e.getKey(), normalizeValue(e.getValue())); + } + return out; + } else if (val instanceof List in) { + List out = new ArrayList<>(in.size()); + for (Object item : in) { + out.add(normalizeValue(item)); + } + return out; + } + return val; + } + + private static boolean isDocList(Map m) { + return m.get("numFound") instanceof Number && m.get("docs") instanceof List; + } + + private static boolean isNestedDoc(Map m) { + return m.containsKey("_nest_path_") || m.containsKey("_nest_parent_"); + } + + @SuppressWarnings("unchecked") + private static SolrDocumentList toDocList(Map m) { + SolrDocumentList docs = new SolrDocumentList(); + docs.setNumFound(((Number) m.get("numFound")).longValue()); + if (m.get("start") instanceof Number start) { + docs.setStart(start.longValue()); + } + if (m.get("maxScore") instanceof Number maxScore) { + docs.setMaxScore(maxScore.floatValue()); + } + if (m.get("numFoundExact") instanceof Boolean exact) { + docs.setNumFoundExact(exact); + } + for (Object d : (List) m.get("docs")) { + docs.add(toDoc(d)); + } + return docs; + } + + @SuppressWarnings("unchecked") + private static SolrDocument toDoc(Object o) { + SolrDocument doc = new SolrDocument(); + if (o instanceof Map) { + for (Map.Entry f : ((Map) o).entrySet()) { + if (CommonParams.CHILDDOC.equals(f.getKey()) && f.getValue() instanceof List kids) { + // JSON has no document type, so nested documents arrive as a field holding a list of + // maps. The other parsers hand them back as child documents, so this one does too. + for (Object kid : kids) { + doc.addChildDocument(toDoc(kid)); + } + continue; + } + // The value may be a reconstructed SolrDocumentList, which addField would unwrap. + doc.setField(f.getKey(), normalizeValue(f.getValue())); + } + } + return doc; + } +} diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java index 9884d8a1ef57..e33255f16e76 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseParser.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.Locale; import java.util.Set; +import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; /** @@ -50,6 +51,20 @@ private boolean validateContentTypes() { /** The writer type placed onto the request as the {@code wt} param. */ public abstract String getWriterType(); // for example: wt=XML, JSON, etc + /** + * Params this parser requires on the request in order to read the response, applied alongside + * {@code wt}. + * + *

These take precedence over the request's own params, as {@code wt} does: a parser that + * cannot read the form the caller asked for would fail rather than honour it. The JSON map parser + * requires {@code json.nl=map}, since a NamedList written any other way cannot be reconstructed. + * + * @return the params to apply, or null if the parser needs nothing beyond {@code wt} + */ + public SolrParams getAdditionalRequestParams() { + return null; + } + public abstract NamedList processResponse(InputStream body, String encoding) throws IOException; @@ -66,4 +81,19 @@ public abstract NamedList processResponse(InputStream body, String encod * @return the MIME types that this parser is capable of parsing. Never null. */ public abstract Set getContentTypes(); + + /** + * Parses the response and returns it in the canonical shape the SolrJ response classes expect: a + * {@link NamedList} tree with {@link org.apache.solr.common.SolrDocumentList} for document + * sections. + * + *

Most parsers produce that shape directly and inherit this method unchanged. A parser whose + * natural output is a raw structure of {@code Map}s and {@code List}s — such as the JSON map + * parser — overrides it to convert, so that the conversion is the parser's own responsibility + * rather than something a client has to know to apply. + */ + public NamedList processCanonicalResponse(InputStream body, String encoding) + throws IOException { + return processResponse(body, encoding); + } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/SolrResponseBase.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/SolrResponseBase.java index 9d90184ce429..86f883b2c781 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/SolrResponseBase.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/SolrResponseBase.java @@ -92,7 +92,9 @@ public NamedList getResponseHeader() { public int getStatus() { NamedList header = getResponseHeader(); if (header != null) { - return (Integer) header.get("status"); + // ResponseParsers vary in the numeric type they produce (e.g. JSON yields Long), so widen + // via Number rather than casting to Integer. See SOLR-17316. + return ((Number) header.get("status")).intValue(); } else { return 0; } @@ -101,7 +103,7 @@ public int getStatus() { public int getQTime() { NamedList header = getResponseHeader(); if (header != null) { - return (Integer) header.get("QTime"); + return ((Number) header.get("QTime")).intValue(); } else { return 0; } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/SpellCheckResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/SpellCheckResponse.java index 4d3a077da510..ca6c056b8422 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/SpellCheckResponse.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/SpellCheckResponse.java @@ -144,10 +144,10 @@ public Suggestion(String token, NamedList suggestion) { suggestion.forEach( (n, val) -> { switch (n) { - case "numFound" -> numFound = (Integer) val; - case "startOffset" -> startOffset = (Integer) val; - case "endOffset" -> endOffset = (Integer) val; - case "origFreq" -> originalFrequency = (Integer) val; + case "numFound" -> numFound = ((Number) val).intValue(); + case "startOffset" -> startOffset = ((Number) val).intValue(); + case "endOffset" -> endOffset = ((Number) val).intValue(); + case "origFreq" -> originalFrequency = ((Number) val).intValue(); case "suggestion" -> { List list = (List) val; if (!list.isEmpty() && list.get(0) instanceof NamedList) { @@ -157,7 +157,7 @@ public Suggestion(String token, NamedList suggestion) { alternativeFrequencies = new ArrayList<>(); for (NamedList nl : extended) { alternatives.add((String) nl.get("word")); - alternativeFrequencies.add((Integer) nl.get("freq")); + alternativeFrequencies.add(((Number) nl.get("freq")).intValue()); } } else { @SuppressWarnings("unchecked") diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java index 2eb2d376e7ef..c880357a5f29 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java @@ -23,9 +23,12 @@ import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.Set; +import org.apache.solr.client.solrj.response.ResponseNormalizer; import org.apache.solr.client.solrj.response.ResponseParser; import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.IOUtils; +import org.apache.solr.common.util.JsonTextWriter; import org.apache.solr.common.util.NamedList; import org.noggit.JSONParser; import org.noggit.ObjectBuilder; @@ -63,4 +66,24 @@ public NamedList processResponse(InputStream body, String encoding) thro public Set getContentTypes() { return CONTENT_TYPES; } + + private static final SolrParams REQUEST_PARAMS = + SolrParams.of(JsonTextWriter.JSON_NL_STYLE, JsonTextWriter.JSON_NL_MAP); + + /** + * Asks for {@code json.nl=map}, so that a {@link NamedList} written by the server arrives as a + * JSON object and {@link #processCanonicalResponse} can restore it as a {@code NamedList}. Under + * the default {@code json.nl=flat} the keys and values are flattened into one array, and the + * structure cannot be recovered. + */ + @Override + public SolrParams getAdditionalRequestParams() { + return REQUEST_PARAMS; + } + + @Override + public NamedList processCanonicalResponse(InputStream body, String encoding) + throws IOException { + return ResponseNormalizer.normalize(processResponse(body, encoding)); + } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java index 7f34859f0a31..35344e78a190 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/schema/SchemaResponse.java @@ -157,7 +157,8 @@ private static String getSchemaName(@SuppressWarnings({"rawtypes"}) Map schemaNa } private static Float getSchemaVersion(@SuppressWarnings({"rawtypes"}) Map schemaNamedList) { - return (Float) schemaNamedList.get("version"); + Object v = schemaNamedList.get("version"); + return v == null ? null : ((Number) v).floatValue(); } private static String getSchemaUniqueKey(@SuppressWarnings({"rawtypes"}) Map schemaNamedList) { diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleJsonMapTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleJsonMapTest.java new file mode 100644 index 000000000000..caf45ca5b57b --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleJsonMapTest.java @@ -0,0 +1,32 @@ +/* + * 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; + +import org.apache.solr.SolrTestCaseJ4.SuppressSSL; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; + +/** Runs the example tests over {@link JsonMapResponseParser}. */ +@SuppressSSL(bugUrl = "https://issues.apache.org/jira/browse/SOLR-5776") +public class SolrExampleJsonMapTest extends SolrExampleTests { + @Override + public SolrClient createNewSolrClient() { + return solrTestRule + .newSolrClientBuilder() + .withResponseParser(new JsonMapResponseParser()) + .build(); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java index 64090fb8e179..1d1d10fa82fe 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java @@ -783,12 +783,12 @@ public void testAugmentFields() throws Exception { SolrDocument out2 = out.get(1); assertEquals("111", out1.getFieldValue("id")); assertEquals("222", out2.getFieldValue("id")); - assertEquals(1.0f, out1.getFieldValue("score")); - assertEquals(1.0f, out2.getFieldValue("score")); + assertEquals(1.0, ((Number) out1.getFieldValue("score")).doubleValue(), 0.0); + assertEquals(1.0, ((Number) out2.getFieldValue("score")).doubleValue(), 0.0); // check that the docid is one bigger - int id1 = (Integer) out1.getFieldValue("[docid]"); - int id2 = (Integer) out2.getFieldValue("[docid]"); + int id1 = ((Number) out1.getFieldValue("[docid]")).intValue(); + int id2 = ((Number) out2.getFieldValue("[docid]")).intValue(); assertTrue("should be bigger [" + id1 + "," + id2 + "]", id2 > id1); // The score from explain should be the same as the score @@ -797,7 +797,7 @@ public void testAugmentFields() throws Exception { // Augmented _value_ with alias assertEquals("aaa", out1.get("aaa")); - assertEquals(10, ((Integer) out1.get("ten")).intValue()); + assertEquals(10, ((Number) out1.get("ten")).intValue()); } @Test @@ -1915,7 +1915,7 @@ public void testPivotFacetsRanges() throws Exception { List list = rsp.getFacetRanges(); assertEquals(2, list.size()); @SuppressWarnings("unchecked") - RangeFacet range1 = list.get(0); + RangeFacet range1 = list.get(0); assertEquals("price1", range1.getName()); assertEquals(0, range1.getStart().intValue()); assertEquals(200, range1.getEnd().intValue()); @@ -1931,7 +1931,7 @@ public void testPivotFacetsRanges() throws Exception { assertEquals(0, counts1.get(3).getCount()); assertEquals("150.0", counts1.get(3).getValue()); @SuppressWarnings("unchecked") - RangeFacet range2 = list.get(1); + RangeFacet range2 = list.get(1); assertEquals("price2", range2.getName()); assertEquals(0, range2.getStart().intValue()); assertEquals(200, range2.getEnd().intValue()); @@ -1958,9 +1958,9 @@ public void testPivotFacetsRanges() throws Exception { for (RangeFacet range : featuresBBBRanges) { if (range.getName().equals("price1")) { assertNotNull(range); - assertEquals(0, ((Float) range.getStart()).intValue()); - assertEquals(200, ((Float) range.getEnd()).intValue()); - assertEquals(50, ((Float) range.getGap()).intValue()); + assertEquals(0, ((Number) range.getStart()).intValue()); + assertEquals(200, ((Number) range.getEnd()).intValue()); + assertEquals(50, ((Number) range.getGap()).intValue()); @SuppressWarnings({"unchecked"}) List counts = range.getCounts(); assertEquals(4, counts.size()); @@ -1982,9 +1982,9 @@ public void testPivotFacetsRanges() throws Exception { } } else if (range.getName().equals("price2")) { assertNotNull(range); - assertEquals(0, ((Float) range.getStart()).intValue()); - assertEquals(200, ((Float) range.getEnd()).intValue()); - assertEquals(50, ((Float) range.getGap()).intValue()); + assertEquals(0, ((Number) range.getStart()).intValue()); + assertEquals(200, ((Number) range.getEnd()).intValue()); + assertEquals(50, ((Number) range.getGap()).intValue()); @SuppressWarnings({"unchecked"}) List counts = range.getCounts(); assertEquals(4, counts.size()); @@ -2014,9 +2014,9 @@ public void testPivotFacetsRanges() throws Exception { for (RangeFacet range : facetRanges) { if (range.getName().equals("price1")) { assertNotNull(range); - assertEquals(0, ((Float) range.getStart()).intValue()); - assertEquals(200, ((Float) range.getEnd()).intValue()); - assertEquals(50, ((Float) range.getGap()).intValue()); + assertEquals(0, ((Number) range.getStart()).intValue()); + assertEquals(200, ((Number) range.getEnd()).intValue()); + assertEquals(50, ((Number) range.getGap()).intValue()); @SuppressWarnings({"unchecked"}) List counts = range.getCounts(); assertEquals(4, counts.size()); @@ -2038,9 +2038,9 @@ public void testPivotFacetsRanges() throws Exception { } } else if (range.getName().equals("price2")) { assertNotNull(range); - assertEquals(0, ((Float) range.getStart()).intValue()); - assertEquals(200, ((Float) range.getEnd()).intValue()); - assertEquals(50, ((Float) range.getGap()).intValue()); + assertEquals(0, ((Number) range.getStart()).intValue()); + assertEquals(200, ((Number) range.getEnd()).intValue()); + assertEquals(50, ((Number) range.getGap()).intValue()); @SuppressWarnings({"unchecked"}) List counts = range.getCounts(); assertEquals(4, counts.size()); @@ -2374,7 +2374,7 @@ public void testUpdateField() throws Exception { assertEquals("Doc count does not match", 1, resp.getResults().getNumFound()); Long version = (Long) resp.getResults().get(0).getFirstValue("_version_"); assertNotNull("no version returned", version); - assertEquals(1.0f, resp.getResults().get(0).getFirstValue(field)); + assertEquals(1.0, ((Number) resp.getResults().get(0).getFirstValue(field)).doubleValue(), 0.0); // update "price" with incorrect version (optimistic locking) HashMap oper = new HashMap<>(); // need better api for this??? @@ -2420,7 +2420,11 @@ public void testUpdateField() throws Exception { client.commit(); resp = client.query(q); assertEquals("Doc count does not match", 1, resp.getResults().getNumFound()); - assertEquals("price was not updated?", 100.0f, resp.getResults().get(0).getFirstValue(field)); + assertEquals( + "price was not updated?", + 100.0, + ((Number) resp.getResults().get(0).getFirstValue(field)).doubleValue(), + 0.0); assertEquals("no name?", "gadget", resp.getResults().get(0).getFirstValue("name")); // update "price", no version @@ -2432,7 +2436,11 @@ public void testUpdateField() throws Exception { client.commit(); resp = client.query(q); assertEquals("Doc count does not match", 1, resp.getResults().getNumFound()); - assertEquals("price was not updated?", 200.0f, resp.getResults().get(0).getFirstValue(field)); + assertEquals( + "price was not updated?", + 200.0, + ((Number) resp.getResults().get(0).getFirstValue(field)).doubleValue(), + 0.0); assertEquals("no name?", "gadget", resp.getResults().get(0).getFirstValue("name")); } @@ -2611,7 +2619,10 @@ public void testChildDocTransformer() throws IOException, SolrServerException { for (SolrDocument kid : outDoc.getChildDocuments()) { String kidId = (String) kid.getFieldValue("id"); - assertEquals("kid is the wrong level", kidLevel, (int) kid.getFieldValue("level_i")); + assertEquals( + "kid is the wrong level", + kidLevel, + ((Number) kid.getFieldValue("level_i")).intValue()); SolrInputDocument origChild = findDescendant(origDoc, kidId); assertNotNull(docId + " doesn't have descendant " + kidId, origChild); } @@ -2694,7 +2705,7 @@ public void testChildDocTransformer() throws IOException, SolrServerException { assertTrue("orig doc had no kids at all", origDoc.hasChildDocuments()); for (SolrDocument kid : outDoc.getChildDocuments()) { String kidId = (String) kid.getFieldValue("id"); - int kidLevel = (int) kid.getFieldValue("level_i"); + int kidLevel = ((Number) kid.getFieldValue("level_i")).intValue(); assertTrue( "kid level to high: " + kidLevelMax + "<" + kidLevel, kidLevel <= kidLevelMax); assertTrue( diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/AdminResponseNumericTypeTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/AdminResponseNumericTypeTest.java new file mode 100644 index 000000000000..7722a3ca8c67 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/AdminResponseNumericTypeTest.java @@ -0,0 +1,105 @@ +/* + * 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.response; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.Map; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.response.schema.SchemaResponse; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; +import org.junit.Test; + +/** + * Non-binary parsers deliver integers as Long. These response classes widen via Number rather than + * casting to Integer/Long/Float, so a Long value must not throw (SOLR-17316). Each assertion fails + * with a ClassCastException without the widening. + */ +public class AdminResponseNumericTypeTest extends SolrTestCase { + + /** AnalysisResponseBase.buildTokenInfo: start/end/position widened from Number. */ + @Test + public void testAnalysisTokenInfo() { + NamedList token = new SimpleOrderedMap<>(); + token.add("text", "foo"); + token.add("start", 1L); // JSON yields Long + token.add("end", 4L); + token.add("position", 2L); + + var probe = + new AnalysisResponseBase() { + TokenInfo build(NamedList nl) { + return buildTokenInfo(nl); + } + }; + AnalysisResponseBase.TokenInfo info = probe.build(token); + assertEquals(1, info.getStart()); + assertEquals(4, info.getEnd()); + assertEquals(2, info.getPosition()); + } + + /** LukeResponse.getMaxDoc/getNumTerms: widened from Number. */ + @Test + public void testLukeIndexInfo() { + NamedList index = new SimpleOrderedMap<>(); + index.add("maxDoc", 10L); // JSON yields Long + index.add("numTerms", 42L); + NamedList body = new SimpleOrderedMap<>(); + body.add("index", index); + + LukeResponse r = new LukeResponse(); + r.setResponse(body); + assertEquals(Integer.valueOf(10), r.getMaxDoc()); + assertEquals(Integer.valueOf(42), r.getNumTerms()); + } + + /** LukeResponse.FieldInfo.distinct: widened from Number. */ + @Test + public void testLukeFieldDistinct() { + NamedList field = new SimpleOrderedMap<>(); + field.add("type", "string"); + field.add("distinct", 5L); // JSON yields Long + NamedList fields = new SimpleOrderedMap<>(); + fields.add("cat", field); + NamedList body = new SimpleOrderedMap<>(); + body.add("fields", fields); + + LukeResponse r = new LukeResponse(); + r.setResponse(body); + assertEquals(5, r.getFieldInfo("cat").getDistinct()); + } + + /** SchemaResponse.getSchemaVersion: widened from Number (JSON yields Double for 1.6). */ + @Test + @SuppressWarnings({"unchecked", "rawtypes"}) + public void testSchemaVersion() { + Map schema = new LinkedHashMap(); + schema.put("version", 1.6d); // JSON yields Double + schema.put("fields", new ArrayList<>()); + schema.put("dynamicFields", new ArrayList<>()); + schema.put("fieldTypes", new ArrayList<>()); + schema.put("copyFields", new ArrayList<>()); + NamedList body = new SimpleOrderedMap<>(); + body.add("schema", schema); + + SchemaResponse r = new SchemaResponse(); + r.setResponse(body); + Float version = r.getSchemaRepresentation().getVersion(); + assertEquals(1.6f, version.floatValue(), 0.0001f); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseJsonParserIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseJsonParserIntegrationTest.java new file mode 100644 index 000000000000..a0bd24bcf0e6 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseJsonParserIntegrationTest.java @@ -0,0 +1,96 @@ +/* + * 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.response; + +import static org.apache.solr.SolrTestCaseJ4.sdoc; + +import java.util.List; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.impl.HttpJdkSolrClient; +import org.apache.solr.client.solrj.request.SolrQuery; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; +import org.apache.solr.util.ExternalPaths; +import org.apache.solr.util.SolrJettyTestRule; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +/** + * End-to-end: a real HTTP query with the JSON map response parser must return a fully typed + * QueryResponse, proving SolrRequest.process() normalizes the non-canonical JSON response at the + * boundary before the response classes read it (SOLR-17316). + */ +public class QueryResponseJsonParserIntegrationTest extends SolrTestCase { + + @ClassRule public static SolrJettyTestRule solrTestRule = new SolrJettyTestRule(); + + @BeforeClass + public static void beforeClass() throws Exception { + System.setProperty("solr.security.allow.paths", "*"); + solrTestRule.startSolr(); + solrTestRule.newCollection().withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET).create(); + + SolrClient client = solrTestRule.getSolrClient(); + client.add( + List.of( + sdoc("id", "1", "cat", "electronics"), + sdoc("id", "2", "cat", "electronics"), + sdoc("id", "3", "cat", "books"))); + client.commit(); + } + + /** The default (Jetty) transport. */ + @Test + public void testTypedQueryResponseOverJsonJetty() throws Exception { + try (SolrClient client = + solrTestRule + .newSolrClientBuilder() + .withResponseParser(new JsonMapResponseParser()) + .build()) { + assertTypedResponse(client); + } + } + + /** The JDK transport shares the same response boundary, so it must behave identically. */ + @Test + public void testTypedQueryResponseOverJsonJdk() throws Exception { + try (SolrClient client = + new HttpJdkSolrClient.Builder(solrTestRule.getBaseUrl()) + .withResponseParser(new JsonMapResponseParser()) + .build()) { + assertTypedResponse(client); + } + } + + private void assertTypedResponse(SolrClient client) throws Exception { + SolrQuery q = new SolrQuery("*:*"); + q.setRows(10); + q.addFacetField("cat"); + // no json.nl here: the parser supplies the style it can read + + QueryResponse rsp = client.query("collection1", q); + + assertEquals(0, rsp.getStatus()); + assertEquals(3, rsp.getResults().getNumFound()); + assertNotNull(rsp.getResults().get(0).getFirstValue("id")); + + FacetField cat = rsp.getFacetField("cat"); + assertNotNull("facet field cat", cat); + assertEquals(2, cat.getValueCount()); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseSectionParityTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseSectionParityTest.java new file mode 100644 index 000000000000..1d384958ad7c --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseSectionParityTest.java @@ -0,0 +1,177 @@ +/* + * 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.response; + +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; +import org.apache.solr.common.util.NamedList; +import org.junit.Test; + +/** + * Each test feeds a JSON (json.nl=map) response for one QueryResponse section through the + * normalizer and asserts the typed accessor works. Sections with a numeric cast (grouping, facets, + * spellcheck) also guard the Number widening; the rest guard the structural Map -> NamedList / + * SolrDocumentList reconstruction the section relies on. + */ +public class QueryResponseSectionParityTest extends SolrTestCase { + + private static QueryResponse parse(String json) throws Exception { + NamedList parsed = + new JsonMapResponseParser() + .processResponse( + new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)), "UTF-8"); + QueryResponse r = new QueryResponse(); + r.setResponse(ResponseNormalizer.normalize(parsed)); + return r; + } + + private static final String HEADER = + """ + "responseHeader":{"status":0,"QTime":1},"""; + + /** pivot facets: count is an Integer cast (QueryResponse readPivots). */ + @Test + public void testPivotFacets() throws Exception { + String json = + "{" + + HEADER + + """ + "facet_counts":{"facet_queries":{},"facet_fields":{}, + "facet_pivot":{"cat":[{"field":"cat","value":"electronics","count":3}]}}}"""; + QueryResponse r = parse(json); + assertNotNull("facetPivot", r.getFacetPivot()); + assertEquals(3, r.getFacetPivot().get("cat").get(0).getCount()); + } + + /** grouping: matches / ngroups are Integer casts (QueryResponse extractGroupedInfo). */ + @Test + public void testGrouping() throws Exception { + String json = + "{" + + HEADER + + """ + "grouped":{"cat":{"matches":3,"ngroups":2,"groups":[ + {"groupValue":"a","doclist":{"numFound":2,"start":0,"docs":[{"id":"1"}]}}, + {"groupValue":"b","doclist":{"numFound":1,"start":0,"docs":[{"id":"2"}]}} + ]}}}"""; + QueryResponse r = parse(json); + GroupResponse gr = r.getGroupResponse(); + assertNotNull("groupResponse", gr); + assertEquals(3, gr.getValues().get(0).getMatches()); + assertEquals(Integer.valueOf(2), gr.getValues().get(0).getNGroups()); + } + + /** interval facets: count is an Integer cast (QueryResponse extractFacetInfo). */ + @Test + public void testIntervalFacets() throws Exception { + String json = + "{" + + HEADER + + """ + "facet_counts":{"facet_queries":{},"facet_fields":{}, + "facet_intervals":{"price":{"[0,10]":5,"[11,100]":3}}}}"""; + QueryResponse r = parse(json); + assertNotNull("intervalFacets", r.getIntervalFacets()); + assertEquals(2, r.getIntervalFacets().get(0).getIntervals().size()); + assertEquals(5, r.getIntervalFacets().get(0).getIntervals().get(0).getCount()); + } + + /** field stats: count/missing (Long) and sumOfSquares/stddev (Double) casts (FieldStatsInfo). */ + @Test + public void testFieldStats() throws Exception { + String json = + "{" + + HEADER + + """ + "stats":{"stats_fields":{"price":{ + "min":9.0,"max":12.0,"count":2,"missing":0, + "sumOfSquares":225.0,"stddev":1.5,"countDistinct":2,"cardinality":2}}}}"""; + QueryResponse r = parse(json); + assertNotNull("fieldStatsInfo", r.getFieldStatsInfo()); + FieldStatsInfo price = r.getFieldStatsInfo().get("price"); + assertNotNull("price stats", price); + assertEquals(Long.valueOf(2), price.getCount()); + assertEquals(Long.valueOf(0), price.getMissing()); + assertEquals(Double.valueOf(1.5), price.getStddev()); + assertEquals(Long.valueOf(2), price.getCardinality()); + } + + /** spellcheck: numFound / startOffset / origFreq are Integer casts (SpellCheckResponse). */ + @Test + public void testSpellCheck() throws Exception { + String json = + "{" + + HEADER + + """ + "spellcheck":{"suggestions":{ + "helo":{"numFound":1,"startOffset":0,"endOffset":4,"origFreq":0, + "suggestion":[{"word":"hello","freq":5}]}}}}"""; + QueryResponse r = parse(json); + SpellCheckResponse sc = r.getSpellCheckResponse(); + assertNotNull("spellcheck", sc); + SpellCheckResponse.Suggestion s = sc.getSuggestion("helo"); + assertNotNull("suggestion", s); + assertEquals(1, s.getNumFound()); + assertEquals(0, s.getStartOffset()); + assertEquals(Integer.valueOf(5), s.getAlternativeFrequencies().get(0)); + } + + /** highlighting: no numeric cast, but exercises Map->NamedList reconstruction over JSON. */ + @Test + public void testHighlighting() throws Exception { + String json = + "{" + + HEADER + + """ + "highlighting":{"1":{"name":["foo"]}}}"""; + QueryResponse r = parse(json); + assertNotNull("highlighting", r.getHighlighting()); + assertEquals("foo", r.getHighlighting().get("1").get("name").get(0)); + } + + /** terms: df/ttf are read via Number, and the section is a nested NamedList over JSON. */ + @Test + public void testTerms() throws Exception { + String json = + "{" + + HEADER + + """ + "terms":{"cat":{"electronics":3,"books":1}}}"""; + QueryResponse r = parse(json); + assertNotNull("termsResponse", r.getTermsResponse()); + assertEquals(2, r.getTermsResponse().getTerms("cat").size()); + assertEquals(3L, r.getTermsResponse().getTerms("cat").get(0).getFrequency()); + } + + /** + * moreLikeThis: each value is a {numFound,docs} object -> must reconstruct as SolrDocumentList. + */ + @Test + public void testMoreLikeThis() throws Exception { + String json = + "{" + + HEADER + + """ + "moreLikeThis":{"1":{"numFound":1,"start":0,"docs":[{"id":"2"}]}}}"""; + QueryResponse r = parse(json); + assertNotNull("moreLikeThis", r.getMoreLikeThis()); + assertEquals(1, r.getMoreLikeThis().get("1").getNumFound()); + assertEquals("2", r.getMoreLikeThis().get("1").get(0).getFirstValue("id")); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseNormalizerTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseNormalizerTest.java new file mode 100644 index 000000000000..e4c047a3135f --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseNormalizerTest.java @@ -0,0 +1,347 @@ +/* + * 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.response; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.apache.solr.SolrTestCase; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.params.CommonParams; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; +import org.junit.Test; + +/** Intensive tests for {@link ResponseNormalizer}. */ +public class ResponseNormalizerTest extends SolrTestCase { + + @Test + public void testNullAndEmpty() { + assertNull(ResponseNormalizer.normalize(null)); + assertEquals(0, ResponseNormalizer.normalize(new NamedList<>()).size()); + } + + @Test + public void testAlreadyCanonicalPassesThrough() { + NamedList header = new SimpleOrderedMap<>(); + header.add("status", 0); + NamedList in = new SimpleOrderedMap<>(); + in.add("responseHeader", header); + + NamedList out = ResponseNormalizer.normalize(in); + assertTrue(out.get("responseHeader") instanceof NamedList); + assertEquals(0, ((NamedList) out.get("responseHeader")).get("status")); + } + + @Test + public void testMapBecomesNamedListRecursively() { + Map inner = new LinkedHashMap<>(); + inner.put("a", 1); + Map mid = new LinkedHashMap<>(); + mid.put("inner", inner); + NamedList in = new NamedList<>(); + in.add("mid", mid); + + NamedList out = ResponseNormalizer.normalize(in); + Object midOut = out.get("mid"); + assertTrue("mid should be NamedList", midOut instanceof NamedList); + Object innerOut = ((NamedList) midOut).get("inner"); + assertTrue("inner should be NamedList", innerOut instanceof NamedList); + assertEquals(1, ((NamedList) innerOut).get("a")); + } + + @Test + public void testDocListReconstruction() { + Map doc1 = new LinkedHashMap<>(); + doc1.put("id", "1"); + Map response = new LinkedHashMap<>(); + response.put("numFound", 5L); + response.put("start", 0L); + response.put("maxScore", 1.5); + response.put("numFoundExact", false); + response.put("docs", new ArrayList<>(List.of(doc1))); + NamedList in = new NamedList<>(); + in.add("response", response); + + NamedList out = ResponseNormalizer.normalize(in); + Object r = out.get("response"); + assertTrue("response should be SolrDocumentList", r instanceof SolrDocumentList); + SolrDocumentList docs = (SolrDocumentList) r; + assertEquals(5L, docs.getNumFound()); + assertEquals(0L, docs.getStart()); + assertEquals(Float.valueOf(1.5f), docs.getMaxScore()); + assertFalse("numFoundExact must survive the conversion", docs.getNumFoundExact()); + assertEquals(1, docs.size()); + assertEquals("1", docs.get(0).getFirstValue("id")); + } + + @Test + public void testEmptyDocList() { + Map response = new LinkedHashMap<>(); + response.put("numFound", 0L); + response.put("docs", new ArrayList<>()); + NamedList in = new NamedList<>(); + in.add("response", response); + + SolrDocumentList docs = (SolrDocumentList) ResponseNormalizer.normalize(in).get("response"); + assertEquals(0L, docs.getNumFound()); + assertTrue(docs.isEmpty()); + } + + @Test + public void testDocListValuedFieldIsReconstructed() { + // a doc field whose value is itself a {numFound,docs} object becomes a nested SolrDocumentList + Map child = new LinkedHashMap<>(); + child.put("id", "child-1"); + Map childList = new LinkedHashMap<>(); + childList.put("numFound", 1L); + childList.put("docs", new ArrayList<>(List.of(child))); + + Map parent = new LinkedHashMap<>(); + parent.put("id", "parent-1"); + parent.put("nested", childList); + + Map response = new LinkedHashMap<>(); + response.put("numFound", 1L); + response.put("docs", new ArrayList<>(List.of(parent))); + NamedList in = new NamedList<>(); + in.add("response", response); + + SolrDocumentList docs = (SolrDocumentList) ResponseNormalizer.normalize(in).get("response"); + SolrDocument parentDoc = docs.get(0); + Object nested = parentDoc.getFieldValue("nested"); + assertTrue("nested docList field reconstructed", nested instanceof SolrDocumentList); + assertEquals("child-1", ((SolrDocumentList) nested).get(0).getFirstValue("id")); + } + + /** + * A nested-document schema stamps every child with {@code _nest_path_}, and {@code [child]} + * returns it under {@code fl=*}, so a named child says what it is. The shapes here are the ones a + * live response carries: a single child under its own field name, an array of children under + * theirs, and a grandchild inside the single child. The binary and XML parsers hand all three + * back as documents ({@code } in XML), so this one must too. + */ + @Test + public void testNamedNestedDocumentsAreReconstructed() { + Map grandChild = new LinkedHashMap<>(); + grandChild.put("id", "3"); + grandChild.put("test2_s", "secondTest"); + grandChild.put("_nest_path_", "/lonely#/lonelyGrandChild#"); + + Map lonely = new LinkedHashMap<>(); + lonely.put("id", "2"); + lonely.put("test_s", "testing"); + lonely.put("_nest_path_", "/lonely#"); + lonely.put("lonelyGrandChild", grandChild); + + Map topping = new LinkedHashMap<>(); + topping.put("id", "4"); + topping.put("type_s", "Regular"); + topping.put("_nest_path_", "/toppings#0"); + + Map parent = new LinkedHashMap<>(); + parent.put("id", "1"); + parent.put("lonely", lonely); + parent.put("toppings", new ArrayList<>(List.of(topping))); + + Map response = new LinkedHashMap<>(); + response.put("numFound", 1L); + response.put("docs", new ArrayList<>(List.of(parent))); + NamedList in = new NamedList<>(); + in.add("response", response); + + SolrDocument parentDoc = + ((SolrDocumentList) ResponseNormalizer.normalize(in).get("response")).get(0); + + Object single = parentDoc.getFieldValue("lonely"); + assertTrue("a named child must be a SolrDocument, not a map", single instanceof SolrDocument); + assertEquals("testing", ((SolrDocument) single).getFirstValue("test_s")); + + Object nestedGrandChild = ((SolrDocument) single).getFieldValue("lonelyGrandChild"); + assertTrue("a grandchild must be reconstructed too", nestedGrandChild instanceof SolrDocument); + + Object array = parentDoc.getFieldValue("toppings"); + assertTrue("a named child array stays a List", array instanceof List); + assertTrue( + "its elements must be SolrDocuments", ((List) array).get(0) instanceof SolrDocument); + + // Named children are field values, not child documents -- the same as binary and XML, where + // ChildDocTransformer calls setField for a named path and addChildDocuments only for anonymous. + assertFalse( + "a named child is a field value, so the parent has no child documents", + parentDoc.hasChildDocuments()); + } + + /** An unmarked object stays a map: most map-valued fields in a response are not documents. */ + @Test + public void testUnmarkedObjectIsNotPromotedToDocument() { + Map notADoc = new LinkedHashMap<>(); + notADoc.put("id", "2"); + notADoc.put("test_s", "testing"); + + Map parent = new LinkedHashMap<>(); + parent.put("id", "1"); + parent.put("someStruct", notADoc); + + Map response = new LinkedHashMap<>(); + response.put("numFound", 1L); + response.put("docs", new ArrayList<>(List.of(parent))); + NamedList in = new NamedList<>(); + in.add("response", response); + + SolrDocument parentDoc = + ((SolrDocumentList) ResponseNormalizer.normalize(in).get("response")).get(0); + assertTrue( + "an object with no nest marker must stay a NamedList", + parentDoc.getFieldValue("someStruct") instanceof NamedList); + } + + @Test + public void testListOfMapsNormalized() { + Map a = new LinkedHashMap<>(); + a.put("x", 1); + Map b = new LinkedHashMap<>(); + b.put("y", 2); + NamedList in = new NamedList<>(); + in.add("things", new ArrayList<>(Arrays.asList(a, b))); + + NamedList out = ResponseNormalizer.normalize(in); + List things = (List) out.get("things"); + assertTrue(things.get(0) instanceof NamedList); + assertEquals(1, ((NamedList) things.get(0)).get("x")); + } + + @Test + public void testMixedNumberTypesPreserved() { + // normalizer preserves numeric values as-is (widening happens at the getter layer) + Map header = new LinkedHashMap<>(); + header.put("status", 0L); // JSON Long + header.put("QTime", 7L); + NamedList in = new NamedList<>(); + in.add("responseHeader", header); + + NamedList out = ResponseNormalizer.normalize(in); + NamedList h = (NamedList) out.get("responseHeader"); + assertEquals(0L, h.get("status")); + assertEquals(7L, h.get("QTime")); + } + + @Test + public void testNotADocListWhenNumFoundMissing() { + // a map with "docs" but no numeric numFound is NOT a doc list -> stays a NamedList + Map notDocs = new LinkedHashMap<>(); + notDocs.put("docs", new ArrayList<>()); + NamedList in = new NamedList<>(); + in.add("x", notDocs); + + assertTrue(ResponseNormalizer.normalize(in).get("x") instanceof NamedList); + } + + /** + * A plain {@link NamedList} must not be promoted to a {@link SimpleOrderedMap}. The two are + * written differently — a JSON writer renders a SimpleOrderedMap as {@code {"foo":10}} and a + * NamedList as {@code ["foo",10]} — and SimpleOrderedMap also implements {@link java.util.Map}, + * whose contract assumes unique keys that a general NamedList does not guarantee. Normalizing + * must preserve the concrete type rather than widen it. + */ + public void testPlainNamedListIsNotPromotedToMap() { + NamedList plain = new NamedList<>(); + plain.add("dup", 1); + plain.add("dup", 2); + + NamedList in = new SimpleOrderedMap<>(); + in.add("section", plain); + + Object out = ResponseNormalizer.normalize(in).get("section"); + assertTrue("must stay a NamedList", out instanceof NamedList); + assertFalse( + "a plain NamedList must not become a SimpleOrderedMap", out instanceof SimpleOrderedMap); + + // and the repeated key survives, which is the reason the distinction matters + NamedList outList = (NamedList) out; + assertEquals(2, outList.size()); + assertEquals("dup", outList.getName(0)); + assertEquals("dup", outList.getName(1)); + assertEquals(1, outList.getVal(0)); + assertEquals(2, outList.getVal(1)); + } + + /** A SimpleOrderedMap stays one: it is what the binary parser produces and extractors cast to. */ + public void testSimpleOrderedMapStaysOne() { + NamedList inner = new SimpleOrderedMap<>(); + inner.add("a", 1); + + NamedList in = new SimpleOrderedMap<>(); + in.add("section", inner); + + Object out = ResponseNormalizer.normalize(in).get("section"); + assertTrue("must stay a SimpleOrderedMap", out instanceof SimpleOrderedMap); + } + + /** + * JSON conveys nested documents as a {@code _childDocuments_} field holding a list of maps; the + * binary and XML parsers hand them back as child documents, so this one must too. + */ + @Test + public void testChildDocumentsAreReconstructed() { + Map kid = new LinkedHashMap<>(); + kid.put("id", "kid1"); + Map parent = new LinkedHashMap<>(); + parent.put("id", "parent1"); + parent.put(CommonParams.CHILDDOC, List.of(kid)); + Map docList = new LinkedHashMap<>(); + docList.put("numFound", 1); + docList.put("docs", List.of(parent)); + NamedList in = new SimpleOrderedMap<>(); + in.add("response", docList); + + SolrDocumentList out = (SolrDocumentList) ResponseNormalizer.normalize(in).get("response"); + SolrDocument outParent = out.get(0); + assertTrue("child documents must be reconstructed", outParent.hasChildDocuments()); + assertEquals(1, outParent.getChildDocumentCount()); + assertEquals("kid1", outParent.getChildDocuments().get(0).getFieldValue("id")); + assertNull( + "the raw field must not remain alongside the children", + outParent.getFieldValue(CommonParams.CHILDDOC)); + } + + /** Children nest, so a grandchild must be reconstructed too. */ + @Test + public void testChildDocumentsNest() { + Map grandkid = new LinkedHashMap<>(); + grandkid.put("id", "grandkid1"); + Map kid = new LinkedHashMap<>(); + kid.put("id", "kid1"); + kid.put(CommonParams.CHILDDOC, List.of(grandkid)); + Map parent = new LinkedHashMap<>(); + parent.put("id", "parent1"); + parent.put(CommonParams.CHILDDOC, List.of(kid)); + Map docList = new LinkedHashMap<>(); + docList.put("numFound", 1); + docList.put("docs", List.of(parent)); + NamedList in = new SimpleOrderedMap<>(); + in.add("response", docList); + + SolrDocumentList out = (SolrDocumentList) ResponseNormalizer.normalize(in).get("response"); + SolrDocument outKid = out.get(0).getChildDocuments().get(0); + assertTrue("grandchildren must be reconstructed", outKid.hasChildDocuments()); + assertEquals("grandkid1", outKid.getChildDocuments().get(0).getFieldValue("id")); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalResponseTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalResponseTest.java new file mode 100644 index 000000000000..3b96a85a2d0b --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalResponseTest.java @@ -0,0 +1,81 @@ +/* + * 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.response; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.util.Map; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.util.NamedList; +import org.junit.Test; + +/** + * Pins the {@link ResponseParser#processCanonicalResponse} contract: whatever a parser's natural + * output looks like, this method returns the canonical shape the SolrJ response classes read — a + * NamedList tree with SolrDocumentList for document sections. The conversion belongs to the parser, + * so a client does not need to know which parsers require it. + */ +public class ResponseParserCanonicalResponseTest extends SolrTestCase { + + private static final String JSON = + """ + {"responseHeader":{"status":0,"QTime":1},\ + "response":{"numFound":1,"start":0,"numFoundExact":true,"docs":[{"id":"1"}]}}"""; + + private static InputStream json() { + return new ByteArrayInputStream(JSON.getBytes(UTF_8)); + } + + /** The JSON map parser's own output is raw: Maps where the response classes expect NamedLists. */ + @Test + public void testJsonMapParserRawOutputIsNotCanonical() throws Exception { + NamedList raw = new JsonMapResponseParser().processResponse(json(), null); + assertTrue("raw header should be a Map", raw.get("responseHeader") instanceof Map); + assertFalse( + "raw header should not be a NamedList", raw.get("responseHeader") instanceof NamedList); + assertFalse( + "raw response should not be a SolrDocumentList", + raw.get("response") instanceof SolrDocumentList); + } + + /** ... and processCanonicalResponse converts it, without the caller asking. */ + @Test + public void testJsonMapParserCanonicalResponseIsConverted() throws Exception { + NamedList out = new JsonMapResponseParser().processCanonicalResponse(json(), null); + assertTrue("header must be a NamedList", out.get("responseHeader") instanceof NamedList); + assertTrue( + "response must be a SolrDocumentList", out.get("response") instanceof SolrDocumentList); + assertEquals(1, ((SolrDocumentList) out.get("response")).getNumFound()); + } + + /** Parsers that are canonical already inherit the default and are unchanged by it. */ + @Test + public void testCanonicalParsersPassThrough() throws Exception { + String xml = + """ + + 0"""; + NamedList out = + new XMLResponseParser() + .processCanonicalResponse(new ByteArrayInputStream(xml.getBytes(UTF_8)), null); + assertTrue("header must be a NamedList", out.get("responseHeader") instanceof NamedList); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/SolrResponseBaseTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/SolrResponseBaseTest.java new file mode 100644 index 000000000000..ff4a16b3ae07 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/SolrResponseBaseTest.java @@ -0,0 +1,71 @@ +/* + * 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.response; + +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; +import org.junit.Test; + +/** Tests that {@link SolrResponseBase} getters work across ResponseParsers (SOLR-17316). */ +public class SolrResponseBaseTest extends SolrTestCase { + + /** The JSON parser yields a Map header with Long numbers, the case that regressed. */ + @Test + public void testStatusAndQTimeWithJsonParser() throws Exception { + String json = "{\"responseHeader\":{\"status\":0,\"QTime\":7}}"; + NamedList parsed = + new JsonMapResponseParser() + .processResponse( + new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)), "UTF-8"); + + SolrResponseBase response = new SolrResponseBase(); + response.setResponse(parsed); + + assertEquals(0, response.getStatus()); + assertEquals(7, response.getQTime()); + } + + /** The binary parser yields a NamedList header with Integer numbers (the original happy path). */ + @Test + public void testStatusAndQTimeWithBinaryStyleHeader() { + NamedList header = new SimpleOrderedMap<>(); + header.add("status", 0); + header.add("QTime", 7); + NamedList body = new SimpleOrderedMap<>(); + body.add("responseHeader", header); + + SolrResponseBase response = new SolrResponseBase(); + response.setResponse(body); + + assertEquals(0, response.getStatus()); + assertEquals(7, response.getQTime()); + } + + /** With no responseHeader the getters return 0 rather than throwing. */ + @Test + public void testStatusAndQTimeWithNoHeader() { + SolrResponseBase response = new SolrResponseBase(); + response.setResponse(new SimpleOrderedMap<>()); + + assertEquals(0, response.getStatus()); + assertEquals(0, response.getQTime()); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestSuggesterResponse.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestSuggesterResponse.java index 713470c4bff7..84f4d7b95a31 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestSuggesterResponse.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/TestSuggesterResponse.java @@ -26,6 +26,7 @@ import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.request.SolrQuery; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.util.EnvUtils; @@ -138,11 +139,17 @@ private void addSampleDocs() throws SolrServerException, IOException { } /* - * Randomizes the ResponseParser to test that both javabin and xml responses parse correctly. See SOLR-15070 + * Randomizes the ResponseParser so that every wt the response classes are expected to work with is + * exercised: javabin and xml (SOLR-15070), and the JSON map parser, whose raw Maps are converted to + * the canonical shape by the parser itself (SOLR-17316). */ private SolrClient createSuggestSolrClient() { final ResponseParser randomParser = - random().nextBoolean() ? new JavaBinResponseParser() : new XMLResponseParser(); + switch (random().nextInt(3)) { + case 0 -> new JavaBinResponseParser(); + case 1 -> new XMLResponseParser(); + default -> new JsonMapResponseParser(); + }; return solrTestRule.newSolrClientBuilder().withResponseParser(randomParser).build(); } }