This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-mcp.git
The following commit(s) were added to refs/heads/main by this push:
new 9b1d0e7 fix(config): keep empty facet arrays as NamedList, not List
(#175)
9b1d0e7 is described below
commit 9b1d0e798c785a7bdff512dbb912c1a0b7b8d8de
Author: Aditya Parikh <[email protected]>
AuthorDate: Fri Sep 11 10:03:40 2026 -0400
fix(config): keep empty facet arrays as NamedList, not List (#175)
* fix(config): keep empty facet arrays as NamedList, not List
JsonResponseParser classified arrays purely by shape: an array of
[String, non-String, ...] pairs became a NamedList, anything else a List.
An empty array has no shape to inspect, so a facet on a field that matched
zero documents was converted to an empty List.
SolrJ's QueryResponse casts every facet_counts/facet_fields entry to
NamedList, so that produced a ClassCastException whenever a faceted field
had no matches - a plausible query, not an edge case.
Give the traversal positional context instead of guessing: thread the node
path through toNamedList/convertValue and treat anything directly under
facet_counts/facet_fields as a NamedList regardless of shape. The shape
heuristic still covers other flat-NamedList sites.
Adds JsonResponseParserTest, which was confirmed to fail without this change
(2 of its 5 cases) and to pass with it, including an end-to-end case that
feeds the parsed response into a real QueryResponse.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Signed-off-by: Aditya Parikh <[email protected]>
* test(search): cover the empty facet end to end against real Solr
JsonResponseParserTest pins the decoding at the parser boundary using a
hand-written payload. That leaves one assumption untested: that a real Solr
actually emits [] for a facet on a zero-hit query. If Solr ever emitted {}
instead, the unit tests would keep passing while the bug they guard no
longer
matched reality.
Adds the end-to-end counterpart via Testcontainers: facet a filter designed
to
match nothing, and assert an empty facet map comes back rather than an
exception.
Verified by reverting only the JsonResponseParser change on this branch and
re-running: the test fails with java.lang.ClassCastException. With the fix
it
passes, and the full build is 378 tests, 0 failures.
Ported from #185, which duplicated this PR and is being closed in its
favour.
Signed-off-by: Aditya Parikh <[email protected]>
---------
Signed-off-by: Aditya Parikh <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../solr/mcp/server/config/JsonResponseParser.java | 51 ++++++---
.../mcp/server/config/JsonResponseParserTest.java | 123 +++++++++++++++++++++
.../search/SearchServiceIntegrationTest.java | 25 +++++
3 files changed, 186 insertions(+), 13 deletions(-)
diff --git
a/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java
b/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java
index 545d5e8..8e50490 100644
--- a/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java
+++ b/src/main/java/org/apache/solr/mcp/server/config/JsonResponseParser.java
@@ -88,22 +88,33 @@ class JsonResponseParser extends ResponseParser {
return List.of(MediaType.APPLICATION_JSON_VALUE,
MediaType.TEXT_PLAIN_VALUE);
}
+ /**
+ * Path of the object whose direct children are per-field facet arrays.
Arrays
+ * found one level below this path are always NamedLists, regardless of
shape.
+ */
+ private static final String FACET_FIELDS_PATH =
"facet_counts/facet_fields";
+
@Override
public NamedList<Object> processResponse(InputStream body, String
encoding) {
try {
- return toNamedList(mapper.readTree(body));
+ return toNamedList(mapper.readTree(body), "");
} catch (IOException e) {
throw new
SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to parse Solr JSON
response", e);
}
}
- private SimpleOrderedMap<Object> toNamedList(JsonNode objectNode) {
+ private SimpleOrderedMap<Object> toNamedList(JsonNode objectNode,
String path) {
SimpleOrderedMap<Object> result = new SimpleOrderedMap<>();
- objectNode.fields().forEachRemaining(entry ->
result.add(entry.getKey(), convertValue(entry.getValue())));
+ objectNode.fields().forEachRemaining(
+ entry -> result.add(entry.getKey(),
convertValue(entry.getValue(), child(path, entry.getKey()))));
return result;
}
- private @Nullable Object convertValue(JsonNode node) {
+ private static String child(String path, String key) {
+ return path.isEmpty() ? key : path + "/" + key;
+ }
+
+ private @Nullable Object convertValue(JsonNode node, String path) {
if (node.isNull())
return null;
if (node.isBoolean())
@@ -117,21 +128,28 @@ class JsonResponseParser extends ResponseParser {
if (node.isDouble() || node.isFloat())
return node.floatValue();
if (node.isObject())
- return convertObject(node);
+ return convertObject(node, path);
if (node.isArray())
- return convertArray(node);
+ return convertArray(node, path);
return node.asText();
}
- private Object convertObject(JsonNode node) {
+ private Object convertObject(JsonNode node, String path) {
// Detect a Solr query result set by the presence of numFound +
docs
if (node.has("numFound") && node.has("docs")) {
return toSolrDocumentList(node);
}
- return toNamedList(node);
+ return toNamedList(node, path);
}
- private Object convertArray(JsonNode arrayNode) {
+ private Object convertArray(JsonNode arrayNode, String path) {
+ // Facet field values are always NamedLists, even when empty.
The shape
+ // heuristic below cannot recognise an empty array, and
returning a List
+ // for a facet on a zero-hit field would break SolrJ's
QueryResponse,
+ // which casts each facet_fields entry to NamedList.
+ if (isFacetFieldValue(path)) {
+ return flatArrayToNamedList(arrayNode);
+ }
// Detect Solr's flat NamedList encoding: [String, non-String,
String,
// non-String, ...]
// Used for facet counts (json.nl=flat default). Distinguished
from plain string
@@ -141,10 +159,16 @@ class JsonResponseParser extends ResponseParser {
return flatArrayToNamedList(arrayNode);
}
List<Object> list = new ArrayList<>(arrayNode.size());
- arrayNode.forEach(element -> list.add(convertValue(element)));
+ arrayNode.forEach(element -> list.add(convertValue(element,
path)));
return list;
}
+ /** True for {@code facet_counts/facet_fields/<fieldName>}. */
+ private static boolean isFacetFieldValue(String path) {
+ int lastSlash = path.lastIndexOf('/');
+ return lastSlash > 0 && path.substring(0,
lastSlash).equals(FACET_FIELDS_PATH);
+ }
+
/**
* Returns true when the array has even length, every even-indexed
element is a
* string (the key), and every odd-indexed element is NOT a string (the
value).
@@ -168,7 +192,8 @@ class JsonResponseParser extends ResponseParser {
private SimpleOrderedMap<Object> flatArrayToNamedList(JsonNode
arrayNode) {
SimpleOrderedMap<Object> result = new SimpleOrderedMap<>();
for (int i = 0; i < arrayNode.size(); i += 2) {
- result.add(arrayNode.get(i).textValue(),
convertValue(arrayNode.get(i + 1)));
+ // Values here are facet counts (scalars), never nested
facet arrays.
+ result.add(arrayNode.get(i).textValue(),
convertValue(arrayNode.get(i + 1), ""));
}
return result;
}
@@ -196,10 +221,10 @@ class JsonResponseParser extends ResponseParser {
if (val.isArray()) {
// Multi-valued field — always a plain list,
never a flat NamedList
List<Object> values = new
ArrayList<>(val.size());
- val.forEach(v -> values.add(convertValue(v)));
+ val.forEach(v -> values.add(convertValue(v,
"")));
doc.setField(entry.getKey(), values);
} else {
- doc.setField(entry.getKey(), convertValue(val));
+ doc.setField(entry.getKey(), convertValue(val,
""));
}
});
return doc;
diff --git
a/src/test/java/org/apache/solr/mcp/server/config/JsonResponseParserTest.java
b/src/test/java/org/apache/solr/mcp/server/config/JsonResponseParserTest.java
new file mode 100644
index 0000000..1b70cb3
--- /dev/null
+++
b/src/test/java/org/apache/solr/mcp/server/config/JsonResponseParserTest.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.mcp.server.config;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.ByteArrayInputStream;
+import java.nio.charset.StandardCharsets;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.util.NamedList;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Unit tests for {@link JsonResponseParser}'s conversion of Solr's JSON wire
+ * format into the {@link NamedList} tree SolrJ expects.
+ *
+ * <p>
+ * The facet cases are regression coverage: SolrJ's {@link QueryResponse} casts
+ * every {@code facet_counts/facet_fields} entry to a {@link NamedList}, so an
+ * empty facet array must not be converted to a {@link java.util.List}.
+ */
+class JsonResponseParserTest {
+
+ private final JsonResponseParser parser = new JsonResponseParser(new
ObjectMapper());
+
+ private NamedList<Object> parse(String json) {
+ return parser.processResponse(new
ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)), "UTF-8");
+ }
+
+ @SuppressWarnings("unchecked")
+ private static NamedList<Object> facetFields(NamedList<Object>
response) {
+ NamedList<Object> facetCounts = (NamedList<Object>)
response.get("facet_counts");
+ return (NamedList<Object>) facetCounts.get("facet_fields");
+ }
+
+ @Test
+ @DisplayName("populated facet array converts to a NamedList of counts")
+ void populatedFacetBecomesNamedList() {
+ NamedList<Object> response = parse("""
+
{"facet_counts":{"facet_fields":{"genre":["fantasy",10,"scifi",5]}}}
+ """);
+
+ Object genre = facetFields(response).get("genre");
+ NamedList<Object> counts = assertInstanceOf(NamedList.class,
genre);
+ assertEquals(2, counts.size());
+ assertEquals(10, counts.get("fantasy"));
+ assertEquals(5, counts.get("scifi"));
+ }
+
+ @Test
+ @DisplayName("empty facet array still converts to a NamedList, not a
List")
+ void emptyFacetBecomesEmptyNamedList() {
+ // A facet on a field where nothing matched. The shape
heuristic cannot
+ // recognise [] as a flat NamedList, so position in the tree
must decide.
+ NamedList<Object> response = parse("""
+ {"facet_counts":{"facet_fields":{"genre":[]}}}
+ """);
+
+ Object genre = facetFields(response).get("genre");
+ NamedList<Object> counts = assertInstanceOf(NamedList.class,
genre);
+ assertEquals(0, counts.size());
+ }
+
+ @Test
+ @DisplayName("empty array outside facet_fields stays a List")
+ void emptyArrayElsewhereStaysList() {
+ NamedList<Object> response = parse("""
+ {"responseHeader":{"warnings":[]}}
+ """);
+
+ @SuppressWarnings("unchecked")
+ NamedList<Object> header = (NamedList<Object>)
response.get("responseHeader");
+ assertInstanceOf(java.util.List.class, header.get("warnings"));
+ }
+
+ @Test
+ @DisplayName("plain string array is not mistaken for a flat NamedList")
+ void plainStringArrayStaysList() {
+ NamedList<Object> response = parse("""
+ {"responseHeader":{"fields":["col1","col2"]}}
+ """);
+
+ @SuppressWarnings("unchecked")
+ NamedList<Object> header = (NamedList<Object>)
response.get("responseHeader");
+ assertInstanceOf(java.util.List.class, header.get("fields"));
+ }
+
+ @Test
+ @DisplayName("QueryResponse can read facets when one field has zero
matches")
+ void queryResponseHandlesEmptyFacet() {
+ // End-to-end guard: this is the cast that used to throw
ClassCastException.
+ NamedList<Object> response = parse("""
+ {"responseHeader":{"status":0,"QTime":1},
+ "response":{"numFound":0,"start":0,"docs":[]},
+
"facet_counts":{"facet_fields":{"genre":[],"author":["asimov",3]}}}
+ """);
+
+ QueryResponse queryResponse = new QueryResponse();
+ queryResponse.setResponse(response);
+
+ assertEquals(2, queryResponse.getFacetFields().size());
+
assertTrue(queryResponse.getFacetField("genre").getValues().isEmpty());
+ assertEquals(1,
queryResponse.getFacetField("author").getValues().size());
+ }
+}
diff --git
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
index bb3c842..99fb607 100644
---
a/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
+++
b/src/test/java/org/apache/solr/mcp/server/search/SearchServiceIntegrationTest.java
@@ -185,6 +185,31 @@ class SearchServiceIntegrationTest {
assertEquals(10, documents.size());
}
+ /**
+ * Zero matches is an ordinary search outcome, not an error. Solr
writes an
+ * empty facet as {@code []}, which must still reach SolrJ as a
NamedList —
+ * {@code QueryResponse.getFacetFields()} casts to one, so a plain list
surfaces
+ * as {@code ClassCastException: ArrayList cannot be cast to
+ * NamedList} instead of an empty result.
+ *
+ * <p>
+ * {@link org.apache.solr.mcp.server.config.JsonResponseParserTest}
pins the
+ * same behaviour at the parser boundary against a hand-written
payload. This
+ * test is the end-to-end counterpart: it proves a real Solr actually
emits
+ * {@code []} for a zero-hit facet, which is the premise the unit tests
assume.
+ */
+ @Test
+ void facetingAQueryThatMatchesNothingReturnsEmptyFacets() throws
SolrServerException, IOException {
+ SearchResponse result = searchService.search(COLLECTION_NAME,
"genre_s:no_such_genre_exists", null,
+ List.of("genre_s"), null, null, 0);
+
+ assertNotNull(result);
+ assertEquals(0, result.numFound(), "the filter is designed to
match nothing");
+ assertNotNull(result.facets(), "facets must be present even
when nothing matched");
+ assertTrue(result.facets().getOrDefault("genre_s",
Map.of()).isEmpty(),
+ () -> "expected no facet buckets, got: " +
result.facets().get("genre_s"));
+ }
+
/**
* Remediation hints classify Solr's error text, which this server
cannot see at
* compile time — the strings are produced by solr-core, and only
solr-solrj is
