From 8cc17bc2e858ad01cfc45ae94ff273e96ef1056d Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Wed, 19 Aug 2026 22:30:38 -0400 Subject: [PATCH] fix(search): decode an empty facet as a NamedList, not a List MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A faceted search whose query matched no documents failed with "ClassCastException: ArrayList cannot be cast to NamedList" instead of returning an empty result. Zero matches is an ordinary search outcome, so this fires on well-typed fields too — any filter that happens to match nothing. Solr writes a facet field as a flat array under json.nl=flat, and writes an empty one as []. JsonResponseParser.isFlatNamedList rejects zero-length arrays, so the value fell through to the plain-list branch and SolrJ's QueryResponse.getFacetFields(), which casts to NamedList, threw. Allowing size == 0 in that heuristic is not a fix: [] is genuinely ambiguous. An empty facet must become a NamedList, while an empty "collections": [] must stay a List, since CollectionService.listCollections() casts it to List — so the naive change just moves the ClassCastException to list-collections against an empty cluster. Since shape cannot distinguish them, use the enclosing key. Arrays directly inside facet_fields, facet_queries and facet_intervals are flat NamedLists by definition, empty or not; every other array keeps the existing heuristic. Not fixed by requesting json.nl=map: ResponseParser exposes no hook for query params and the client builder has no default-params method, so it would mean wrapping every request — and json.nl is global, silently collapsing the duplicate keys a NamedList permits. Known gap, documented in the class javadoc: facet_ranges nests its flat list one level deeper under a counts key, so an empty range facet would still decode as a List. The search tool does not expose range faceting, so nothing reaches that path today. Tests, each watched failing first: - JsonResponseParserFacetTest covers the empty facet, a populated facet, and an empty non-facet array as a regression guard for list-collections - SearchServiceIntegrationTest.facetingAQueryThatMatchesNothingReturnsEmptyFacets reproduces it end to end against real Solr via Testcontainers Closes #182 Signed-off-by: Aditya Parikh --- .../mcp/server/config/JsonResponseParser.java | 50 +++++++- .../config/JsonResponseParserFacetTest.java | 121 ++++++++++++++++++ .../search/SearchServiceIntegrationTest.java | 19 +++ 3 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/apache/solr/mcp/server/config/JsonResponseParserFacetTest.java 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 545d5e87..ee845f08 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 @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Set; import org.apache.solr.client.solrj.response.ResponseParser; import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; @@ -66,9 +67,36 @@ * every odd-indexed element is a non-{@link String} value. This reliably * distinguishes {@code ["term", 5, "term2", 3]} (facet NamedList) from * {@code ["col1", "col2"]} (plain string list). + * + *

+ * Why shape alone is not enough: an empty array is + * ambiguous. Solr writes a facet with no buckets as {@code []}, and also writes + * an ordinary empty list — {@code "collections": []} from the Collections API — + * as {@code []}. The first must decode to a {@link NamedList} because SolrJ's + * {@code QueryResponse.getFacetFields()} casts to one; the second must stay a + * {@link List} because {@code CollectionAdminResponse} callers cast to that. No + * rule based on the array can satisfy both, so the parser uses the enclosing + * key instead: arrays directly inside {@code facet_fields}, + * {@code facet_queries} and {@code facet_intervals} are always NamedLists, + * empty or not, and every other array falls back to the shape heuristic above. + * + *

+ * Known gap: {@code facet_ranges} nests its flat list one + * level deeper, under a {@code counts} key, so an empty range facet would still + * decode as a {@link List}. The {@code search} tool does not expose range + * faceting, so nothing currently reaches that path. */ class JsonResponseParser extends ResponseParser { + /** + * Response keys whose immediate children are always NamedLists of counts, + * regardless of how many buckets came back. Solr writes each child as a flat + * array under {@code json.nl=flat}, and writes an empty one as {@code []} — + * which is shape-identical to an ordinary empty list such as + * {@code "collections": []}. Only the enclosing key distinguishes them. + */ + private static final Set FACET_CONTAINERS = Set.of("facet_fields", "facet_queries", "facet_intervals"); + private final ObjectMapper mapper; JsonResponseParser(ObjectMapper mapper) { @@ -99,7 +127,27 @@ public NamedList processResponse(InputStream body, String encoding) { private SimpleOrderedMap toNamedList(JsonNode objectNode) { SimpleOrderedMap result = new SimpleOrderedMap<>(); - objectNode.fields().forEachRemaining(entry -> result.add(entry.getKey(), convertValue(entry.getValue()))); + objectNode.fields() + .forEachRemaining(entry -> result.add(entry.getKey(), + FACET_CONTAINERS.contains(entry.getKey()) && entry.getValue().isObject() + ? toFacetContainer(entry.getValue()) + : convertValue(entry.getValue()))); + return result; + } + + /** + * Converts a facet container, forcing every array child to a NamedList. + * + *

+ * Inside these containers an array is a flat NamedList by definition, so the + * {@link #isFlatNamedList} heuristic is neither needed nor safe: it rejects + * empty arrays, which would hand SolrJ an {@code ArrayList} where + * {@code QueryResponse.getFacetFields()} casts to {@code NamedList}. + */ + private SimpleOrderedMap toFacetContainer(JsonNode containerNode) { + SimpleOrderedMap result = new SimpleOrderedMap<>(); + containerNode.fields().forEachRemaining(entry -> result.add(entry.getKey(), + entry.getValue().isArray() ? flatArrayToNamedList(entry.getValue()) : convertValue(entry.getValue()))); return result; } diff --git a/src/test/java/org/apache/solr/mcp/server/config/JsonResponseParserFacetTest.java b/src/test/java/org/apache/solr/mcp/server/config/JsonResponseParserFacetTest.java new file mode 100644 index 00000000..1d9f3ae4 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/config/JsonResponseParserFacetTest.java @@ -0,0 +1,121 @@ +/* + * 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 com.fasterxml.jackson.databind.ObjectMapper; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import java.util.List; +import org.apache.solr.common.util.NamedList; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for how {@link JsonResponseParser} decodes Solr's facet payloads. + * + *

+ * SolrJ's {@code QueryResponse.getFacetFields()} casts each facet field's value + * to {@link NamedList}. Solr's JSON writer emits a facet field as a flat array + * ({@code json.nl=flat}), and crucially emits an empty facet as + * {@code []} — indistinguishable, by shape alone, from an ordinary empty list + * such as {@code "collections": []}. Deciding by shape therefore cannot be + * correct for both; the parser has to use the enclosing key as context. + */ +class JsonResponseParserFacetTest { + + private static final String EMPTY_FACET_RESPONSE = """ + { + "responseHeader": { "status": 0, "QTime": 3 }, + "response": { "numFound": 0, "start": 0, "docs": [] }, + "facet_counts": { + "facet_queries": {}, + "facet_fields": { "platform": [] }, + "facet_ranges": {}, + "facet_intervals": {}, + "facet_heatmaps": {} + } + } + """; + + private static final String POPULATED_FACET_RESPONSE = """ + { + "responseHeader": { "status": 0, "QTime": 3 }, + "response": { "numFound": 27, "start": 0, "docs": [] }, + "facet_counts": { + "facet_queries": {}, + "facet_fields": { "platform": ["Netflix", 20, "HBO Max", 7] }, + "facet_ranges": {}, + "facet_intervals": {}, + "facet_heatmaps": {} + } + } + """; + + private static final String EMPTY_COLLECTION_LIST_RESPONSE = """ + { + "responseHeader": { "status": 0, "QTime": 1 }, + "collections": [] + } + """; + + private NamedList parse(String json) { + return new JsonResponseParser(new ObjectMapper()) + .processResponse(new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)), "UTF-8"); + } + + private Object facetField(NamedList response, String field) { + NamedList facetCounts = (NamedList) response.get("facet_counts"); + NamedList facetFields = (NamedList) facetCounts.get("facet_fields"); + return facetFields.get(field); + } + + @Test + void facetFieldWithNoBucketsParsesAsNamedList() { + Object platform = facetField(parse(EMPTY_FACET_RESPONSE), "platform"); + + // SolrJ casts this to NamedList; a List here throws ClassCastException + // for any faceted query whose filter happens to match zero documents. + NamedList buckets = assertInstanceOf(NamedList.class, platform, + "An empty facet field must decode as a NamedList, not a List"); + assertEquals(0, buckets.size(), "An empty facet field has no buckets"); + } + + @Test + void facetFieldWithBucketsParsesAsNamedList() { + Object platform = facetField(parse(POPULATED_FACET_RESPONSE), "platform"); + + NamedList buckets = assertInstanceOf(NamedList.class, platform, + "A populated facet field must decode as a NamedList"); + assertEquals(2, buckets.size(), "Two facet buckets were returned"); + assertEquals(20, buckets.get("Netflix"), "Bucket counts survive decoding"); + assertEquals(7, buckets.get("HBO Max"), "Bucket counts survive decoding"); + } + + @Test + void emptyTopLevelArrayStaysAList() { + Object collections = parse(EMPTY_COLLECTION_LIST_RESPONSE).get("collections"); + + // CollectionService.listCollections() casts this to List. Treating + // every empty array as a NamedList would move the ClassCastException here, + // breaking list-collections against an empty cluster. + List names = assertInstanceOf(List.class, collections, + "An empty non-facet array must stay a List so list-collections keeps working"); + assertEquals(0, names.size(), "No collections exist"); + } +} 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 bb3c842c..98274141 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,25 @@ void testBasicSearch() throws SolrServerException, IOException { assertEquals(10, documents.size()); } + /** + * Zero matches is an ordinary search outcome, not an error. Solr writes an + * empty facet as {@code []}, which the response parser must still decode as a + * NamedList — SolrJ's {@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. + */ + @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