From e40d0f58f5bc849422f5d00957ff1ea6aa79b97c Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Wed, 15 Jul 2026 19:10:51 +0300 Subject: [PATCH 01/11] SOLR-17316: fix SolrResponseBase.getStatus/getQTime with non-binary parsers getStatus() and getQTime() cast the header value to Integer, which threw a ClassCastException under a parser that yields a different numeric type (the JSON parser yields Long). Widen via Number.intValue() instead. --- .../solrj/response/SolrResponseBase.java | 6 +- .../solrj/response/SolrResponseBaseTest.java | 71 +++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/SolrResponseBaseTest.java 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/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()); + } +} From 06a193c1f7a9173e492b2eceeaf67743ab26a9dc Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Wed, 15 Jul 2026 19:11:06 +0300 Subject: [PATCH 02/11] SOLR-17316: normalize non-binary responses to the canonical shape The SolrJ response classes assume the Java types the binary parser produces, so reading a response parsed by a non-binary parser (e.g. the JSON map parser) threw ClassCastException: JSON yields raw Map/List where the code expects NamedList/SolrDocumentList, and Long where it casts to Integer. - ResponseNormalizer converts a parsed response into the canonical shape (nested objects -> NamedList/SimpleOrderedMap, a {numFound,docs} object -> SolrDocumentList); it is a no-op for already-canonical binary/XML responses. - ResponseParser.producesCanonicalForm() gates it; JsonMapResponseParser returns false. HttpSolrClient normalizes at the shared response boundary, covering both the JDK and Jetty transports while binary/XML pay nothing. - Remaining numeric reads that cast to Integer/Float are widened via Number (grouping, interval and pivot facet counts, spellcheck, analysis token offsets, Luke, schema version). Tests cover the normalizer, cross-format parity (binary/XML/json-map), each affected section, and an end-to-end HTTP query with the JSON parser on both transports. --- .../SOLR-17316-response-parsers.yml | 12 ++ .../client/solrj/impl/HttpSolrClient.java | 4 + .../solrj/response/AnalysisResponseBase.java | 6 +- .../client/solrj/response/LukeResponse.java | 8 +- .../client/solrj/response/QueryResponse.java | 14 +- .../client/solrj/response/ResponseParser.java | 13 ++ .../solrj/response/SpellCheckResponse.java | 10 +- .../response/json/JsonMapResponseParser.java | 5 + .../solrj/response/schema/SchemaResponse.java | 3 +- .../solr/common/util/ResponseNormalizer.java | 123 +++++++++++++ .../AdminResponseNumericTypeTest.java | 104 +++++++++++ .../QueryResponseCrossFormatTest.java | 144 +++++++++++++++ ...ueryResponseJsonParserIntegrationTest.java | 95 ++++++++++ .../QueryResponseSectionParityTest.java | 162 +++++++++++++++++ .../ResponseParserCanonicalFormTest.java | 41 +++++ .../common/util/ResponseNormalizerTest.java | 169 ++++++++++++++++++ 16 files changed, 895 insertions(+), 18 deletions(-) create mode 100644 changelog/unreleased/SOLR-17316-response-parsers.yml create mode 100644 solr/solrj/src/java/org/apache/solr/common/util/ResponseNormalizer.java create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/AdminResponseNumericTypeTest.java create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseJsonParserIntegrationTest.java create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseSectionParityTest.java create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalFormTest.java create mode 100644 solr/solrj/src/test/org/apache/solr/common/util/ResponseNormalizerTest.java 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/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..e86b881c6f7c 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 @@ -50,6 +50,7 @@ import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.ContentStream; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.ResponseNormalizer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -229,6 +230,9 @@ protected NamedList processErrorsAndResponse( NamedList rsp; try { rsp = processor.processResponse(is, encoding); + if (!processor.producesCanonicalForm()) { + rsp = ResponseNormalizer.normalize(rsp); + } } 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..c669bca2ddeb 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); @@ -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)); } @@ -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?"; 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..1538cecd4a20 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 @@ -66,4 +66,17 @@ 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(); + + /** + * Whether this parser already produces the canonical response shape the SolrJ response classes + * expect: a {@link NamedList} tree with {@link org.apache.solr.common.SolrDocumentList} for + * document sections. The binary and XML parsers do; a parser that yields raw {@code Map}s and + * {@code List}s (such as the JSON map parser) does not, and its output is normalized before the + * response classes read it. + * + * @return true unless the parser yields a raw, un-typed structure + */ + public boolean producesCanonicalForm() { + return true; + } } 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..6a5939f79115 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 @@ -63,4 +63,9 @@ public NamedList processResponse(InputStream body, String encoding) thro public Set getContentTypes() { return CONTENT_TYPES; } + + @Override + public boolean producesCanonicalForm() { + return false; + } } 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/java/org/apache/solr/common/util/ResponseNormalizer.java b/solr/solrj/src/java/org/apache/solr/common/util/ResponseNormalizer.java new file mode 100644 index 000000000000..18f7a42b4068 --- /dev/null +++ b/solr/solrj/src/java/org/apache/solr/common/util/ResponseNormalizer.java @@ -0,0 +1,123 @@ +/* + * 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.common.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.common.SolrDocumentList; + +/** + * 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 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) { + // Already canonical (binary/XML), but its children may still need normalizing. + NamedList in = (NamedList) val; + SimpleOrderedMap out = new SimpleOrderedMap<>(in.size()); + for (Map.Entry e : in) { + out.add(e.getKey(), normalizeValue(e.getValue())); + } + return out; + } else if (val instanceof Map) { + Map m = (Map) val; + if (isDocList(m)) { + return toDocList(m); + } + // SimpleOrderedMap (not plain NamedList): it is the canonical map type the binary parser + // produces, and some response extractors cast to it specifically. + 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) { + List in = (List) val; + 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; + } + + @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()) { + // setField (not addField): addField unwraps a Collection value into a plain list, which + // would drop the type of a reconstructed SolrDocumentList held as a field value. + doc.setField(f.getKey(), normalizeValue(f.getValue())); + } + } + return doc; + } +} 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..6218efcbc8f7 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/AdminResponseNumericTypeTest.java @@ -0,0 +1,104 @@ +/* + * 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.LinkedHashMap; +import java.util.Map; +import org.apache.solr.SolrTestCase; +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 java.util.ArrayList<>()); + schema.put("dynamicFields", new java.util.ArrayList<>()); + schema.put("fieldTypes", new java.util.ArrayList<>()); + schema.put("copyFields", new java.util.ArrayList<>()); + NamedList body = new SimpleOrderedMap<>(); + body.add("schema", schema); + + org.apache.solr.client.solrj.response.schema.SchemaResponse r = + new org.apache.solr.client.solrj.response.schema.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/QueryResponseCrossFormatTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java new file mode 100644 index 000000000000..594833c09716 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java @@ -0,0 +1,144 @@ +/* + * 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.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.util.JavaBinCodec; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.ResponseNormalizer; +import org.apache.solr.common.util.SimpleOrderedMap; +import org.junit.Test; + +/** + * Proves QueryResponse reaches identical typed results whether the wire format was binary or JSON + * (json.nl=map), once the response is passed through {@link ResponseNormalizer}. Binary is the + * canonical baseline; JSON must match it. + */ +public class QueryResponseCrossFormatTest extends SolrTestCase { + + private static NamedList canonical() { + NamedList header = new SimpleOrderedMap<>(); + header.add("status", 0); + header.add("QTime", 7); + + SolrDocumentList docs = new SolrDocumentList(); + docs.setNumFound(2); + docs.setStart(0); + SolrDocument d1 = new SolrDocument(); + d1.addField("id", "1"); + SolrDocument d2 = new SolrDocument(); + d2.addField("id", "2"); + docs.add(d1); + docs.add(d2); + + NamedList catCounts = new SimpleOrderedMap<>(); + catCounts.add("electronics", 3); + catCounts.add("books", 1); + NamedList facetFields = new SimpleOrderedMap<>(); + facetFields.add("cat", catCounts); + NamedList facetCounts = new SimpleOrderedMap<>(); + facetCounts.add("facet_queries", new SimpleOrderedMap<>()); + facetCounts.add("facet_fields", facetFields); + + NamedList body = new SimpleOrderedMap<>(); + body.add("responseHeader", header); + body.add("response", docs); + body.add("facet_counts", facetCounts); + return body; + } + + private static void assertResponse(String fmt, QueryResponse r) { + assertEquals(fmt + " status", 0, r.getStatus()); + assertEquals(fmt + " qtime", 7, r.getQTime()); + assertNotNull(fmt + " results", r.getResults()); + assertEquals(fmt + " numFound", 2, r.getResults().getNumFound()); + assertEquals(fmt + " doc0", "1", r.getResults().get(0).getFirstValue("id")); + + // facet section parity + assertNotNull(fmt + " facetFields", r.getFacetFields()); + assertEquals(fmt + " facet name", "cat", r.getFacetFields().get(0).getName()); + assertEquals(fmt + " facet valueCount", 2, r.getFacetFields().get(0).getValueCount()); + assertEquals(fmt + " facet count", 3L, r.getFacetFields().get(0).getValues().get(0).getCount()); + } + + @Test + @SuppressWarnings("unchecked") + public void testBinaryBaseline() throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (JavaBinCodec codec = new JavaBinCodec()) { + codec.marshal(canonical(), out); + } + NamedList parsed; + try (JavaBinCodec codec = new JavaBinCodec()) { + parsed = (NamedList) codec.unmarshal(new ByteArrayInputStream(out.toByteArray())); + } + QueryResponse r = new QueryResponse(); + r.setResponse(ResponseNormalizer.normalize(parsed)); + assertResponse("binary", r); + } + + @Test + public void testJsonMapMatchesBinary() throws Exception { + String json = + "{\"responseHeader\":{\"status\":0,\"QTime\":7}," + + "\"response\":{\"numFound\":2,\"start\":0," + + "\"docs\":[{\"id\":\"1\"},{\"id\":\"2\"}]}," + + "\"facet_counts\":{\"facet_queries\":{}," + + "\"facet_fields\":{\"cat\":{\"electronics\":3,\"books\":1}}}}"; + NamedList parsed = + new JsonMapResponseParser() + .processResponse( + new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)), "UTF-8"); + QueryResponse r = new QueryResponse(); + r.setResponse(ResponseNormalizer.normalize(parsed)); + assertResponse("json-map", r); + } + + @Test + public void testXmlMatchesBinary() throws Exception { + String xml = + "\n" + + "\n" + + " 0" + + "7\n" + + " \n" + + " 1\n" + + " 2\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " 3" + + "1\n" + + " \n" + + " \n" + + ""; + NamedList parsed = + new XMLResponseParser() + .processResponse( + new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)), "UTF-8"); + QueryResponse r = new QueryResponse(); + r.setResponse(ResponseNormalizer.normalize(parsed)); + assertResponse("xml", r); + } +} 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..72fb78eb6f56 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseJsonParserIntegrationTest.java @@ -0,0 +1,95 @@ +/* + * 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 org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.SolrClient; +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(); + + try (SolrClient client = solrTestRule.getSolrClient()) { + client.add( + java.util.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 org.apache.solr.client.solrj.impl.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"); + q.setParam("json.nl", "map"); // the round-trippable NamedList style + + 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..a822f29dd510 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseSectionParityTest.java @@ -0,0 +1,162 @@ +/* + * 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.ResponseNormalizer; +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/ResponseParserCanonicalFormTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalFormTest.java new file mode 100644 index 000000000000..44a41ad611b3 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalFormTest.java @@ -0,0 +1,41 @@ +/* + * 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 org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; +import org.junit.Test; + +/** + * Pins the producesCanonicalForm() contract that gates response normalization: only the JSON map + * parser (which yields raw Maps/Lists) needs normalizing; the parsers that already produce the + * canonical NamedList/SolrDocumentList shape must report true so they pass through untouched. + */ +public class ResponseParserCanonicalFormTest extends SolrTestCase { + + @Test + public void testCanonicalParsersReportTrue() { + assertTrue(new JavaBinResponseParser().producesCanonicalForm()); + assertTrue(new XMLResponseParser().producesCanonicalForm()); + assertTrue(new InputStreamResponseParser("json").producesCanonicalForm()); + } + + @Test + public void testJsonMapParserReportsFalse() { + assertFalse(new JsonMapResponseParser().producesCanonicalForm()); + } +} diff --git a/solr/solrj/src/test/org/apache/solr/common/util/ResponseNormalizerTest.java b/solr/solrj/src/test/org/apache/solr/common/util/ResponseNormalizerTest.java new file mode 100644 index 000000000000..c3e43d950e1f --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/common/util/ResponseNormalizerTest.java @@ -0,0 +1,169 @@ +/* + * 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.common.util; + +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.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("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()); + 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")); + } + + @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); + } +} From 1b47cd24346aca6e289a2ed9608ba6ad38af34c4 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 21:02:43 +0300 Subject: [PATCH 03/11] =?UTF-8?q?SOLR-17316:=20address=20review=20?= =?UTF-8?q?=E2=80=94=20parser=20owns=20canonicalization,=20and=20package/s?= =?UTF-8?q?tyle=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #4640. The client no longer knows which parsers need normalizing. ResponseParser gains processCanonicalResponse(), which defaults to processResponse() — most parsers are canonical already and inherit it unchanged — and JsonMapResponseParser overrides it to convert. The producesCanonicalForm() predicate is gone, and HttpSolrClient just calls the one method: rsp = processor.processCanonicalResponse(is, encoding); The existing processResponse() is untouched, since 20 call sites use it directly, including the error path in ConcurrentUpdateBaseSolrClient which only reads resp.get("error") and needs no conversion. ResponseNormalizer moves from org.apache.solr.common.util to org.apache.solr.client.solrj.response: it is not a common utility. A plain NamedList is no longer promoted to SimpleOrderedMap. SimpleOrderedMap implements Map and is written differently by the response writers -- a JSON writer renders it as {"foo":10} and a NamedList as ["foo",10] -- so widening the type changes the contract of the value. Only the concrete type is preserved now; a JSON object still becomes a SimpleOrderedMap, since its keys are unique by construction. Worth noting the mutation check: with "always promote" in place, all 27 existing tests passed, so two tests were added for the distinction and they do fail on it. Also: pattern-matching instanceof throughout ResponseNormalizer, text blocks for the JSON literals in the parity test, no FQNs, and no try-with-resources around solrTestRule.getSolrClient() -- its javadoc says "The caller doesn't need to close it". ResponseParserCanonicalFormTest becomes ResponseParserCanonicalResponseTest and pins behaviour rather than the removed predicate: the JSON parser's raw output is Maps, its canonical output is NamedLists and a SolrDocumentList, and a canonical parser passes through unchanged. 200 solrj response/client tests pass; :solr:solrj:check clean. --- .../client/solrj/impl/HttpSolrClient.java | 6 +- .../solrj/response}/ResponseNormalizer.java | 30 ++++--- .../client/solrj/response/ResponseParser.java | 18 +++-- .../response/json/JsonMapResponseParser.java | 6 +- .../QueryResponseCrossFormatTest.java | 1 - ...ueryResponseJsonParserIntegrationTest.java | 16 ++-- .../QueryResponseSectionParityTest.java | 53 +++++++----- .../response}/ResponseNormalizerTest.java | 45 ++++++++++- .../ResponseParserCanonicalFormTest.java | 41 ---------- .../ResponseParserCanonicalResponseTest.java | 81 +++++++++++++++++++ 10 files changed, 200 insertions(+), 97 deletions(-) rename solr/solrj/src/java/org/apache/solr/{common/util => client/solrj/response}/ResponseNormalizer.java (81%) rename solr/solrj/src/test/org/apache/solr/{common/util => client/solrj/response}/ResponseNormalizerTest.java (77%) delete mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalFormTest.java create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalResponseTest.java 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 e86b881c6f7c..d81ac3027b48 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 @@ -50,7 +50,6 @@ import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.ContentStream; import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.ResponseNormalizer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -229,10 +228,7 @@ protected NamedList processErrorsAndResponse( NamedList rsp; try { - rsp = processor.processResponse(is, encoding); - if (!processor.producesCanonicalForm()) { - rsp = ResponseNormalizer.normalize(rsp); - } + 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/common/util/ResponseNormalizer.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseNormalizer.java similarity index 81% rename from solr/solrj/src/java/org/apache/solr/common/util/ResponseNormalizer.java rename to solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseNormalizer.java index 18f7a42b4068..75bbf15799df 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/ResponseNormalizer.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/ResponseNormalizer.java @@ -14,13 +14,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.solr.common.util; +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.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; /** * Converts a parsed response into the canonical shape the SolrJ response classes expect (the shape @@ -54,28 +56,32 @@ private static Object normalizeValue(Object val) { // 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) { - // Already canonical (binary/XML), but its children may still need normalizing. - NamedList in = (NamedList) val; - SimpleOrderedMap out = new SimpleOrderedMap<>(in.size()); - for (Map.Entry e : in) { + } 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) { - Map m = (Map) val; + } else if (val instanceof Map raw) { + Map m = (Map) raw; if (isDocList(m)) { return toDocList(m); } - // SimpleOrderedMap (not plain NamedList): it is the canonical map type the binary parser - // produces, and some response extractors cast to it specifically. + // A JSON object arrives as a Map with unique keys by construction, so SimpleOrderedMap is the + // right target: it is what the binary parser produces, and some response extractors cast to + // it. 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) { - List in = (List) val; + } else if (val instanceof List in) { List out = new ArrayList<>(in.size()); for (Object item : in) { out.add(normalizeValue(item)); 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 1538cecd4a20..222a3cedce4c 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 @@ -68,15 +68,17 @@ public abstract NamedList processResponse(InputStream body, String encod public abstract Set getContentTypes(); /** - * Whether this parser already produces the canonical response shape the SolrJ response classes - * expect: a {@link NamedList} tree with {@link org.apache.solr.common.SolrDocumentList} for - * document sections. The binary and XML parsers do; a parser that yields raw {@code Map}s and - * {@code List}s (such as the JSON map parser) does not, and its output is normalized before the - * response classes read it. + * 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. * - * @return true unless the parser yields a raw, un-typed structure + *

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 boolean producesCanonicalForm() { - return true; + 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/json/JsonMapResponseParser.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/json/JsonMapResponseParser.java index 6a5939f79115..83b5c22b5d58 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,6 +23,7 @@ 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.util.IOUtils; @@ -65,7 +66,8 @@ public Set getContentTypes() { } @Override - public boolean producesCanonicalForm() { - return false; + public NamedList processCanonicalResponse(InputStream body, String encoding) + throws IOException { + return ResponseNormalizer.normalize(processResponse(body, encoding)); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java index 594833c09716..677729214615 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java @@ -25,7 +25,6 @@ import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.util.JavaBinCodec; import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.ResponseNormalizer; import org.apache.solr.common.util.SimpleOrderedMap; import org.junit.Test; 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 index 72fb78eb6f56..dd91b1523a76 100644 --- 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 @@ -18,6 +18,7 @@ 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.request.SolrQuery; @@ -43,14 +44,13 @@ public static void beforeClass() throws Exception { solrTestRule.startSolr(); solrTestRule.newCollection().withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET).create(); - try (SolrClient client = solrTestRule.getSolrClient()) { - client.add( - java.util.List.of( - sdoc("id", "1", "cat", "electronics"), - sdoc("id", "2", "cat", "electronics"), - sdoc("id", "3", "cat", "books"))); - client.commit(); - } + 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. */ 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 index a822f29dd510..1d384958ad7c 100644 --- 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 @@ -21,7 +21,6 @@ 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.ResponseNormalizer; import org.junit.Test; /** @@ -42,7 +41,9 @@ private static QueryResponse parse(String json) throws Exception { return r; } - private static final String HEADER = "\"responseHeader\":{\"status\":0,\"QTime\":1},"; + private static final String HEADER = + """ + "responseHeader":{"status":0,"QTime":1},"""; /** pivot facets: count is an Integer cast (QueryResponse readPivots). */ @Test @@ -50,8 +51,9 @@ public void testPivotFacets() throws Exception { String json = "{" + HEADER - + "\"facet_counts\":{\"facet_queries\":{},\"facet_fields\":{}," - + "\"facet_pivot\":{\"cat\":[{\"field\":\"cat\",\"value\":\"electronics\",\"count\":3}]}}}"; + + """ + "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()); @@ -63,10 +65,11 @@ 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\"}]}}" - + "]}}}"; + + """ + "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); @@ -80,8 +83,9 @@ public void testIntervalFacets() throws Exception { String json = "{" + HEADER - + "\"facet_counts\":{\"facet_queries\":{},\"facet_fields\":{}," - + "\"facet_intervals\":{\"price\":{\"[0,10]\":5,\"[11,100]\":3}}}}"; + + """ + "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()); @@ -94,9 +98,10 @@ 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}}}}"; + + """ + "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"); @@ -113,9 +118,10 @@ public void testSpellCheck() throws Exception { String json = "{" + HEADER - + "\"spellcheck\":{\"suggestions\":{" - + "\"helo\":{\"numFound\":1,\"startOffset\":0,\"endOffset\":4,\"origFreq\":0," - + "\"suggestion\":[{\"word\":\"hello\",\"freq\":5}]}}}}"; + + """ + "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); @@ -129,7 +135,11 @@ public void testSpellCheck() throws Exception { /** highlighting: no numeric cast, but exercises Map->NamedList reconstruction over JSON. */ @Test public void testHighlighting() throws Exception { - String json = "{" + HEADER + "\"highlighting\":{\"1\":{\"name\":[\"foo\"]}}}"; + 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)); @@ -138,7 +148,11 @@ public void testHighlighting() throws Exception { /** 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}}}"; + String json = + "{" + + HEADER + + """ + "terms":{"cat":{"electronics":3,"books":1}}}"""; QueryResponse r = parse(json); assertNotNull("termsResponse", r.getTermsResponse()); assertEquals(2, r.getTermsResponse().getTerms("cat").size()); @@ -153,7 +167,8 @@ public void testMoreLikeThis() throws Exception { String json = "{" + HEADER - + "\"moreLikeThis\":{\"1\":{\"numFound\":1,\"start\":0,\"docs\":[{\"id\":\"2\"}]}}}"; + + """ + "moreLikeThis":{"1":{"numFound":1,"start":0,"docs":[{"id":"2"}]}}}"""; QueryResponse r = parse(json); assertNotNull("moreLikeThis", r.getMoreLikeThis()); assertEquals(1, r.getMoreLikeThis().get("1").getNumFound()); diff --git a/solr/solrj/src/test/org/apache/solr/common/util/ResponseNormalizerTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseNormalizerTest.java similarity index 77% rename from solr/solrj/src/test/org/apache/solr/common/util/ResponseNormalizerTest.java rename to solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseNormalizerTest.java index c3e43d950e1f..417583210010 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/ResponseNormalizerTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseNormalizerTest.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.solr.common.util; +package org.apache.solr.client.solrj.response; import java.util.ArrayList; import java.util.Arrays; @@ -24,6 +24,8 @@ import org.apache.solr.SolrTestCase; import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; import org.junit.Test; /** Intensive tests for {@link ResponseNormalizer}. */ @@ -166,4 +168,45 @@ public void testNotADocListWhenNumFoundMissing() { 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); + } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalFormTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalFormTest.java deleted file mode 100644 index 44a41ad611b3..000000000000 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/response/ResponseParserCanonicalFormTest.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * 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 org.apache.solr.SolrTestCase; -import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; -import org.junit.Test; - -/** - * Pins the producesCanonicalForm() contract that gates response normalization: only the JSON map - * parser (which yields raw Maps/Lists) needs normalizing; the parsers that already produce the - * canonical NamedList/SolrDocumentList shape must report true so they pass through untouched. - */ -public class ResponseParserCanonicalFormTest extends SolrTestCase { - - @Test - public void testCanonicalParsersReportTrue() { - assertTrue(new JavaBinResponseParser().producesCanonicalForm()); - assertTrue(new XMLResponseParser().producesCanonicalForm()); - assertTrue(new InputStreamResponseParser("json").producesCanonicalForm()); - } - - @Test - public void testJsonMapParserReportsFalse() { - assertFalse(new JsonMapResponseParser().producesCanonicalForm()); - } -} 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); + } +} From a33d219104aa28af7bc0b80aa4986648acb6e72a Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 21:26:47 +0300 Subject: [PATCH 04/11] SOLR-17316: randomize wt in TestSuggesterResponse to include the JSON map parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the randomization SOLR-15070 introduced in that test — javabin or xml — to pick among three parsers, so the JSON map parser goes through the same suggester assertions as the other two. It is a regression test for this PR rather than added coverage: on the commit before the normalizer, forcing that parser in fails all three of the test's methods with ClassCastException: class java.util.LinkedHashMap cannot be cast to class org.apache.solr.common.util.NamedList and with the conversion in place they pass. Mutation-checked — removing the conversion from JsonMapResponseParser#processCanonicalResponse brings the ClassCastException back under -Ptests.iters=10. --- .../client/solrj/response/TestSuggesterResponse.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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(); } } From e595e7cea4773d7525e2d5ac2493ea395414e811 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 21:36:26 +0300 Subject: [PATCH 05/11] SOLR-17316: drop QueryResponseCrossFormatTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test fed the JSON parser a literal whose facet_fields was written in json.nl=map form, which JsonMapResponseParser never requests, so the input shape does not occur on a live request — running the response classes end-to-end over a real server surfaced that same section failing as an array. Two of its three methods also asserted existing javabin and xml behaviour rather than anything this change introduces. Coverage of the numeric widening in QueryResponse is kept by QueryResponseSectionParityTest, and of a real request by QueryResponseJsonParserIntegrationTest. --- .../QueryResponseCrossFormatTest.java | 143 ------------------ 1 file changed, 143 deletions(-) delete mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java deleted file mode 100644 index 677729214615..000000000000 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/response/QueryResponseCrossFormatTest.java +++ /dev/null @@ -1,143 +0,0 @@ -/* - * 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.io.ByteArrayOutputStream; -import java.nio.charset.StandardCharsets; -import org.apache.solr.SolrTestCase; -import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; -import org.apache.solr.common.SolrDocument; -import org.apache.solr.common.SolrDocumentList; -import org.apache.solr.common.util.JavaBinCodec; -import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.SimpleOrderedMap; -import org.junit.Test; - -/** - * Proves QueryResponse reaches identical typed results whether the wire format was binary or JSON - * (json.nl=map), once the response is passed through {@link ResponseNormalizer}. Binary is the - * canonical baseline; JSON must match it. - */ -public class QueryResponseCrossFormatTest extends SolrTestCase { - - private static NamedList canonical() { - NamedList header = new SimpleOrderedMap<>(); - header.add("status", 0); - header.add("QTime", 7); - - SolrDocumentList docs = new SolrDocumentList(); - docs.setNumFound(2); - docs.setStart(0); - SolrDocument d1 = new SolrDocument(); - d1.addField("id", "1"); - SolrDocument d2 = new SolrDocument(); - d2.addField("id", "2"); - docs.add(d1); - docs.add(d2); - - NamedList catCounts = new SimpleOrderedMap<>(); - catCounts.add("electronics", 3); - catCounts.add("books", 1); - NamedList facetFields = new SimpleOrderedMap<>(); - facetFields.add("cat", catCounts); - NamedList facetCounts = new SimpleOrderedMap<>(); - facetCounts.add("facet_queries", new SimpleOrderedMap<>()); - facetCounts.add("facet_fields", facetFields); - - NamedList body = new SimpleOrderedMap<>(); - body.add("responseHeader", header); - body.add("response", docs); - body.add("facet_counts", facetCounts); - return body; - } - - private static void assertResponse(String fmt, QueryResponse r) { - assertEquals(fmt + " status", 0, r.getStatus()); - assertEquals(fmt + " qtime", 7, r.getQTime()); - assertNotNull(fmt + " results", r.getResults()); - assertEquals(fmt + " numFound", 2, r.getResults().getNumFound()); - assertEquals(fmt + " doc0", "1", r.getResults().get(0).getFirstValue("id")); - - // facet section parity - assertNotNull(fmt + " facetFields", r.getFacetFields()); - assertEquals(fmt + " facet name", "cat", r.getFacetFields().get(0).getName()); - assertEquals(fmt + " facet valueCount", 2, r.getFacetFields().get(0).getValueCount()); - assertEquals(fmt + " facet count", 3L, r.getFacetFields().get(0).getValues().get(0).getCount()); - } - - @Test - @SuppressWarnings("unchecked") - public void testBinaryBaseline() throws Exception { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - try (JavaBinCodec codec = new JavaBinCodec()) { - codec.marshal(canonical(), out); - } - NamedList parsed; - try (JavaBinCodec codec = new JavaBinCodec()) { - parsed = (NamedList) codec.unmarshal(new ByteArrayInputStream(out.toByteArray())); - } - QueryResponse r = new QueryResponse(); - r.setResponse(ResponseNormalizer.normalize(parsed)); - assertResponse("binary", r); - } - - @Test - public void testJsonMapMatchesBinary() throws Exception { - String json = - "{\"responseHeader\":{\"status\":0,\"QTime\":7}," - + "\"response\":{\"numFound\":2,\"start\":0," - + "\"docs\":[{\"id\":\"1\"},{\"id\":\"2\"}]}," - + "\"facet_counts\":{\"facet_queries\":{}," - + "\"facet_fields\":{\"cat\":{\"electronics\":3,\"books\":1}}}}"; - NamedList parsed = - new JsonMapResponseParser() - .processResponse( - new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)), "UTF-8"); - QueryResponse r = new QueryResponse(); - r.setResponse(ResponseNormalizer.normalize(parsed)); - assertResponse("json-map", r); - } - - @Test - public void testXmlMatchesBinary() throws Exception { - String xml = - "\n" - + "\n" - + " 0" - + "7\n" - + " \n" - + " 1\n" - + " 2\n" - + " \n" - + " \n" - + " \n" - + " \n" - + " 3" - + "1\n" - + " \n" - + " \n" - + ""; - NamedList parsed = - new XMLResponseParser() - .processResponse( - new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)), "UTF-8"); - QueryResponse r = new QueryResponse(); - r.setResponse(ResponseNormalizer.normalize(parsed)); - assertResponse("xml", r); - } -} From fc8239ba430c38bb9a3f3722381c70db20806e0b Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 22:23:27 +0300 Subject: [PATCH 06/11] SOLR-17316: let a ResponseParser supply the request params it needs, and have the JSON map parser ask for json.nl=map JsonMapResponseParser could not read the response it was getting. Under the default json.nl=flat a NamedList is written as an array of alternating names and values, so facet_fields arrived as a List where the response classes expect a NamedList, and the structure cannot be recovered after the fact. The style had to be set by hand on every request, which is not something a caller should have to know per parser. ResponseParser#getRequestParams supplies params alongside wt. Anything the request set explicitly wins, so this only provides defaults. Applied in HttpSolrClient#initializeSolrParams, which every HTTP client routes through. QueryResponseJsonParserIntegrationTest no longer sets json.nl itself, which is what the change is for, and asserts that an explicit value survives. --- .../client/solrj/impl/HttpSolrClient.java | 12 ++++++++ .../client/solrj/response/ResponseParser.java | 12 ++++++++ .../response/json/JsonMapResponseParser.java | 17 +++++++++++ ...ueryResponseJsonParserIntegrationTest.java | 30 ++++++++++++++++++- .../ResponseParserCanonicalResponseTest.java | 22 ++++++++++++++ 5 files changed, 92 insertions(+), 1 deletion(-) 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 d81ac3027b48..1d8dcfec0cc4 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 @@ -27,6 +27,7 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Collection; +import java.util.Iterator; import java.util.Locale; import java.util.Objects; import java.util.Set; @@ -48,6 +49,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; @@ -138,6 +140,16 @@ protected ModifiableSolrParams initializeSolrParams( // The parser 'wt=' param is used instead of the original params ModifiableSolrParams wparams = new ModifiableSolrParams(solrRequest.getParams()); wparams.set(CommonParams.WT, parserToUse.getWriterType()); + // Params the parser needs for its own reads, without overriding what the request already set. + SolrParams parserParams = parserToUse.getRequestParams(); + if (parserParams != null) { + for (Iterator it = parserParams.getParameterNamesIterator(); it.hasNext(); ) { + String name = it.next(); + if (wparams.get(name) == null) { + wparams.set(name, parserParams.getParams(name)); + } + } + } return wparams; } 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 222a3cedce4c..2a5c7fc5bb1b 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,17 @@ 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 needs on the request for its own reads, applied alongside {@code wt}. + * + *

A parser that needs the response written a particular way returns those params here rather + * than relying on callers to set them. Anything the caller set explicitly wins, so this only + * supplies defaults. Returns null when the parser needs nothing beyond {@code wt}. + */ + public SolrParams getRequestParams() { + return null; + } + public abstract NamedList processResponse(InputStream body, String encoding) throws IOException; 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 83b5c22b5d58..ca1fe7c4a46a 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 @@ -26,7 +26,10 @@ 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.MapSolrParams; +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; @@ -65,6 +68,20 @@ public Set getContentTypes() { return CONTENT_TYPES; } + private static final SolrParams REQUEST_PARAMS = + new MapSolrParams(Map.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 getRequestParams() { + return REQUEST_PARAMS; + } + @Override public NamedList processCanonicalResponse(InputStream body, String encoding) throws IOException { 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 index dd91b1523a76..a1c5bbcd9c4f 100644 --- 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 @@ -23,6 +23,9 @@ import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.request.SolrQuery; import org.apache.solr.client.solrj.response.json.JsonMapResponseParser; +import org.apache.solr.common.params.CommonParams; +import org.apache.solr.common.util.JsonTextWriter; +import org.apache.solr.common.util.NamedList; import org.apache.solr.util.ExternalPaths; import org.apache.solr.util.SolrJettyTestRule; import org.junit.BeforeClass; @@ -80,7 +83,7 @@ private void assertTypedResponse(SolrClient client) throws Exception { SolrQuery q = new SolrQuery("*:*"); q.setRows(10); q.addFacetField("cat"); - q.setParam("json.nl", "map"); // the round-trippable NamedList style + // no json.nl here: the parser supplies the style it can read QueryResponse rsp = client.query("collection1", q); @@ -92,4 +95,29 @@ private void assertTypedResponse(SolrClient client) throws Exception { assertNotNull("facet field cat", cat); assertEquals(2, cat.getValueCount()); } + + /** + * The parser only supplies defaults. A caller that sets the param explicitly keeps it — even to a + * value the parser cannot recover, which is the caller's business. + */ + @Test + public void testCallerParamWins() throws Exception { + try (SolrClient client = + solrTestRule + .newSolrClientBuilder() + .withResponseParser(new JsonMapResponseParser()) + .build()) { + SolrQuery q = new SolrQuery("*:*"); + q.setParam(CommonParams.HEADER_ECHO_PARAMS, "all"); + q.setParam(JsonTextWriter.JSON_NL_STYLE, JsonTextWriter.JSON_NL_FLAT); + + QueryResponse rsp = client.query("collection1", q); + + NamedList params = (NamedList) rsp.getResponseHeader().get("params"); + assertEquals( + "an explicit json.nl must not be overwritten", + JsonTextWriter.JSON_NL_FLAT, + params.get(JsonTextWriter.JSON_NL_STYLE)); + } + } } 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 index 3b96a85a2d0b..edb823533622 100644 --- 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 @@ -24,6 +24,8 @@ 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.params.SolrParams; +import org.apache.solr.common.util.JsonTextWriter; import org.apache.solr.common.util.NamedList; import org.junit.Test; @@ -78,4 +80,24 @@ public void testCanonicalParsersPassThrough() throws Exception { .processCanonicalResponse(new ByteArrayInputStream(xml.getBytes(UTF_8)), null); assertTrue("header must be a NamedList", out.get("responseHeader") instanceof NamedList); } + + /** + * A parser that needs the response written a particular way supplies that param itself, rather + * than relying on every caller to know it. The JSON map parser needs {@code json.nl=map}: under + * the default {@code flat} a NamedList arrives as an array of alternating names and values, whose + * structure cannot be recovered. + */ + @Test + public void testJsonMapParserRequestsNlMap() { + SolrParams params = new JsonMapResponseParser().getRequestParams(); + assertNotNull("the JSON map parser must ask for a recoverable NamedList form", params); + assertEquals(JsonTextWriter.JSON_NL_MAP, params.get(JsonTextWriter.JSON_NL_STYLE)); + } + + /** Parsers that need nothing beyond wt contribute no params. */ + @Test + public void testCanonicalParsersRequestNoParams() { + assertNull(new JavaBinResponseParser().getRequestParams()); + assertNull(new XMLResponseParser().getRequestParams()); + } } From 3f2f775883eadc2bb29dda1b7e5d33d222905bc8 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 22:23:36 +0300 Subject: [PATCH 07/11] SOLR-17316: reconstruct child documents from a JSON response JSON has no document type, so the JSON writer emits nested documents as a _childDocuments_ field holding a list of maps. The binary and XML parsers hand them back as child documents; this one left them as a plain field, so SolrDocument#hasChildDocuments was false and the children were unreachable through the documented accessors. Nesting is recursive, so grandchildren are covered too. --- .../solrj/response/ResponseNormalizer.java | 9 ++++ .../response/ResponseNormalizerTest.java | 51 +++++++++++++++++++ 2 files changed, 60 insertions(+) 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 index 75bbf15799df..3e5fb88e27cf 100644 --- 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 @@ -21,6 +21,7 @@ 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; @@ -119,6 +120,14 @@ 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; + } // setField (not addField): addField unwraps a Collection value into a plain list, which // would drop the type of a reconstructed SolrDocumentList held as a field value. doc.setField(f.getKey(), normalizeValue(f.getValue())); 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 index 417583210010..f1f6b0dc64a7 100644 --- 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 @@ -24,6 +24,7 @@ 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; @@ -209,4 +210,54 @@ public void testSimpleOrderedMapStaysOne() { 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")); + } } From ef92060cdcc1c48181c98c0394e20c0340297296 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 22:23:36 +0300 Subject: [PATCH 08/11] SOLR-17316: widen the remaining Integer casts in QueryResponse facet_queries, a range facet's counts and a pivot's query counts were cast to NamedList and iterated as Integer entries, so a response whose numbers arrive as Long threw ClassCastException. Same defect as the accessors already widened here, in three places the earlier commits did not reach; the values are still narrowed to int, so nothing about the public types changes. --- .../client/solrj/response/QueryResponse.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 c669bca2ddeb..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 @@ -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()); } } @@ -403,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); @@ -449,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()); } } } From f3e49862142f82cbcc7c144f8baa5386a12e95bd Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 22:23:45 +0300 Subject: [PATCH 09/11] SOLR-17316: run the example tests over the JSON response parser SolrExampleTests has a subclass per parser and JSON was missing, so the whole 42-test suite now runs against JsonMapResponseParser. That is what found the three defects fixed in the preceding commits; on the commit before them it fails seven ways. Nine assertions in SolrExampleTests pinned the boxed type of a number rather than its value -- (Integer) getFieldValue(..), assertEquals(1.0f, ..), RangeFacet -- and are relaxed to Number where the value is the point. Nothing is ignored: 42 of 42 pass, and the binary, XML, CBOR and HTTP/2 subclasses are unaffected. --- .../client/solrj/SolrExampleJsonMapTest.java | 32 ++++++++++ .../solr/client/solrj/SolrExampleTests.java | 59 +++++++++++-------- 2 files changed, 67 insertions(+), 24 deletions(-) create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleJsonMapTest.java 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( From 25fe626a3cf2e0a70008497e37c5a56d62339e5b Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 22:23:59 +0300 Subject: [PATCH 10/11] SOLR-17316: changelog covers the parser request params and child documents --- changelog/unreleased/SOLR-17316-response-parsers.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/changelog/unreleased/SOLR-17316-response-parsers.yml b/changelog/unreleased/SOLR-17316-response-parsers.yml index 08cf0145ccf6..25f5fb1c3f81 100644 --- a/changelog/unreleased/SOLR-17316-response-parsers.yml +++ b/changelog/unreleased/SOLR-17316-response-parsers.yml @@ -3,7 +3,9 @@ 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. + ClassCastException, and nested documents were unreachable. A ResponseParser can now declare the + request params it needs via getRequestParams(); JsonMapResponseParser uses this to ask for + json.nl=map, so callers no longer have to set it themselves. type: fixed authors: - name: Serhiy Bzhezytskyy From af5676a494da079f042480fc73e2bf4ea6eb619c Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Wed, 5 Aug 2026 16:57:11 +0300 Subject: [PATCH 11/11] SOLR-17316: address review feedback and fix EmbeddedSolrServer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uses SolrParams.wrapDefaults and SolrParams.of instead of hand-rolled merging, and renames getRequestParams to getAdditionalRequestParams. Parser-required params now take precedence over the request's own, as wt already did — a parser that can't read the form the caller asked for would fail rather than honour it. EmbeddedSolrServer called processResponse directly, so a JSON parser there still threw ClassCastException. It now applies the parser's params and reads the response canonically. Named nested documents are reconstructed too, keyed on _nest_path_. --- .../SOLR-17316-response-parsers.yml | 4 +- .../solrj/embedded/EmbeddedSolrServer.java | 3 +- .../TestEmbeddedSolrServerResponseParser.java | 118 ++++++++++++++++++ .../client/solrj/impl/HttpSolrClient.java | 20 +-- .../solrj/response/ResponseNormalizer.java | 19 ++- .../client/solrj/response/ResponseParser.java | 13 +- .../response/json/JsonMapResponseParser.java | 5 +- .../AdminResponseNumericTypeTest.java | 13 +- ...ueryResponseJsonParserIntegrationTest.java | 31 +---- .../response/ResponseNormalizerTest.java | 84 +++++++++++++ .../ResponseParserCanonicalResponseTest.java | 22 ---- 11 files changed, 243 insertions(+), 89 deletions(-) create mode 100644 solr/core/src/test/org/apache/solr/client/solrj/embedded/TestEmbeddedSolrServerResponseParser.java diff --git a/changelog/unreleased/SOLR-17316-response-parsers.yml b/changelog/unreleased/SOLR-17316-response-parsers.yml index 25f5fb1c3f81..08cf0145ccf6 100644 --- a/changelog/unreleased/SOLR-17316-response-parsers.yml +++ b/changelog/unreleased/SOLR-17316-response-parsers.yml @@ -3,9 +3,7 @@ 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, and nested documents were unreachable. A ResponseParser can now declare the - request params it needs via getRequestParams(); JsonMapResponseParser uses this to ask for - json.nl=map, so callers no longer have to set it themselves. + ClassCastException. type: fixed authors: - name: Serhiy Bzhezytskyy 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 1d8dcfec0cc4..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 @@ -27,7 +27,6 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Collection; -import java.util.Iterator; import java.util.Locale; import java.util.Objects; import java.util.Set; @@ -137,20 +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()); - // Params the parser needs for its own reads, without overriding what the request already set. - SolrParams parserParams = parserToUse.getRequestParams(); - if (parserParams != null) { - for (Iterator it = parserParams.getParameterNamesIterator(); it.hasNext(); ) { - String name = it.next(); - if (wparams.get(name) == null) { - wparams.set(name, parserParams.getParams(name)); - } - } - } - 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) { 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 index 3e5fb88e27cf..270c9c0e6a94 100644 --- 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 @@ -34,6 +34,11 @@ * 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 { @@ -74,9 +79,10 @@ private static Object normalizeValue(Object val) { if (isDocList(m)) { return toDocList(m); } - // A JSON object arrives as a Map with unique keys by construction, so SimpleOrderedMap is the - // right target: it is what the binary parser produces, and some response extractors cast to - // it. + 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())); @@ -96,6 +102,10 @@ 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(); @@ -128,8 +138,7 @@ private static SolrDocument toDoc(Object o) { } continue; } - // setField (not addField): addField unwraps a Collection value into a plain list, which - // would drop the type of a reconstructed SolrDocumentList held as a field value. + // The value may be a reconstructed SolrDocumentList, which addField would unwrap. doc.setField(f.getKey(), normalizeValue(f.getValue())); } } 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 2a5c7fc5bb1b..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 @@ -52,13 +52,16 @@ private boolean validateContentTypes() { public abstract String getWriterType(); // for example: wt=XML, JSON, etc /** - * Params this parser needs on the request for its own reads, applied alongside {@code wt}. + * Params this parser requires on the request in order to read the response, applied alongside + * {@code wt}. * - *

A parser that needs the response written a particular way returns those params here rather - * than relying on callers to set them. Anything the caller set explicitly wins, so this only - * supplies defaults. Returns null when the parser needs nothing beyond {@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 getRequestParams() { + public SolrParams getAdditionalRequestParams() { return null; } 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 ca1fe7c4a46a..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 @@ -26,7 +26,6 @@ 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.MapSolrParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.JsonTextWriter; @@ -69,7 +68,7 @@ public Set getContentTypes() { } private static final SolrParams REQUEST_PARAMS = - new MapSolrParams(Map.of(JsonTextWriter.JSON_NL_STYLE, JsonTextWriter.JSON_NL_MAP)); + 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 @@ -78,7 +77,7 @@ public Set getContentTypes() { * structure cannot be recovered. */ @Override - public SolrParams getRequestParams() { + public SolrParams getAdditionalRequestParams() { return REQUEST_PARAMS; } 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 index 6218efcbc8f7..7722a3ca8c67 100644 --- 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 @@ -16,9 +16,11 @@ */ 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; @@ -88,15 +90,14 @@ public void testLukeFieldDistinct() { public void testSchemaVersion() { Map schema = new LinkedHashMap(); schema.put("version", 1.6d); // JSON yields Double - schema.put("fields", new java.util.ArrayList<>()); - schema.put("dynamicFields", new java.util.ArrayList<>()); - schema.put("fieldTypes", new java.util.ArrayList<>()); - schema.put("copyFields", new java.util.ArrayList<>()); + 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); - org.apache.solr.client.solrj.response.schema.SchemaResponse r = - new org.apache.solr.client.solrj.response.schema.SchemaResponse(); + 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 index a1c5bbcd9c4f..a0bd24bcf0e6 100644 --- 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 @@ -21,11 +21,9 @@ 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.common.params.CommonParams; -import org.apache.solr.common.util.JsonTextWriter; -import org.apache.solr.common.util.NamedList; import org.apache.solr.util.ExternalPaths; import org.apache.solr.util.SolrJettyTestRule; import org.junit.BeforeClass; @@ -72,7 +70,7 @@ public void testTypedQueryResponseOverJsonJetty() throws Exception { @Test public void testTypedQueryResponseOverJsonJdk() throws Exception { try (SolrClient client = - new org.apache.solr.client.solrj.impl.HttpJdkSolrClient.Builder(solrTestRule.getBaseUrl()) + new HttpJdkSolrClient.Builder(solrTestRule.getBaseUrl()) .withResponseParser(new JsonMapResponseParser()) .build()) { assertTypedResponse(client); @@ -95,29 +93,4 @@ private void assertTypedResponse(SolrClient client) throws Exception { assertNotNull("facet field cat", cat); assertEquals(2, cat.getValueCount()); } - - /** - * The parser only supplies defaults. A caller that sets the param explicitly keeps it — even to a - * value the parser cannot recover, which is the caller's business. - */ - @Test - public void testCallerParamWins() throws Exception { - try (SolrClient client = - solrTestRule - .newSolrClientBuilder() - .withResponseParser(new JsonMapResponseParser()) - .build()) { - SolrQuery q = new SolrQuery("*:*"); - q.setParam(CommonParams.HEADER_ECHO_PARAMS, "all"); - q.setParam(JsonTextWriter.JSON_NL_STYLE, JsonTextWriter.JSON_NL_FLAT); - - QueryResponse rsp = client.query("collection1", q); - - NamedList params = (NamedList) rsp.getResponseHeader().get("params"); - assertEquals( - "an explicit json.nl must not be overwritten", - JsonTextWriter.JSON_NL_FLAT, - params.get(JsonTextWriter.JSON_NL_STYLE)); - } - } } 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 index f1f6b0dc64a7..e4c047a3135f 100644 --- 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 @@ -75,6 +75,7 @@ public void testDocListReconstruction() { 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); @@ -86,6 +87,7 @@ public void testDocListReconstruction() { 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")); } @@ -129,6 +131,88 @@ public void testDocListValuedFieldIsReconstructed() { 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<>(); 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 index edb823533622..3b96a85a2d0b 100644 --- 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 @@ -24,8 +24,6 @@ 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.params.SolrParams; -import org.apache.solr.common.util.JsonTextWriter; import org.apache.solr.common.util.NamedList; import org.junit.Test; @@ -80,24 +78,4 @@ public void testCanonicalParsersPassThrough() throws Exception { .processCanonicalResponse(new ByteArrayInputStream(xml.getBytes(UTF_8)), null); assertTrue("header must be a NamedList", out.get("responseHeader") instanceof NamedList); } - - /** - * A parser that needs the response written a particular way supplies that param itself, rather - * than relying on every caller to know it. The JSON map parser needs {@code json.nl=map}: under - * the default {@code flat} a NamedList arrives as an array of alternating names and values, whose - * structure cannot be recovered. - */ - @Test - public void testJsonMapParserRequestsNlMap() { - SolrParams params = new JsonMapResponseParser().getRequestParams(); - assertNotNull("the JSON map parser must ask for a recoverable NamedList form", params); - assertEquals(JsonTextWriter.JSON_NL_MAP, params.get(JsonTextWriter.JSON_NL_STYLE)); - } - - /** Parsers that need nothing beyond wt contribute no params. */ - @Test - public void testCanonicalParsersRequestNoParams() { - assertNull(new JavaBinResponseParser().getRequestParams()); - assertNull(new XMLResponseParser().getRequestParams()); - } }