Repository navigation
fix(config): keep empty facet arrays as NamedList, not List - #175
Conversation
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) <noreply@anthropic.com> Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
87a3552 to
1755472
Compare
|
Rebased onto One conflict, in No other changes; the fix and its tests are as reviewed. Verified: |
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 apache#185, which duplicated this PR and is being closed in its favour.
Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
|
Added an end-to-end test and closed a duplicate PR in this one's favour. Duplicate: I opened #185 against the same bug without spotting this PR — my duplicate check Comparing the two implementations, this one is better and is what should merge. It anchors on Ported across (e4d0646): the one thing #185 had that this PR didn't — an integration test The existing Verified rather than assumed: reverting only the |
Resolves the conflicts between main and the Spring Boot 4 / Spring AI 2 upgrade, keeping the Boot 4 line throughout: - build.gradle.kts, libs.versions.toml: keep spring-boot-starter-opentelemetry, mcp-server-security 0.1.14 and the MCP SDK 2.0.1 pin; add CommonMark for markdown indexing (apache#144). JSpecify stays transitive (built into Boot 4). - Service classes: keep the Spring AI 2 annotation package (org.springframework.ai.mcp.annotation) and take the SLF4J loggers from apache#110. - JsonResponseParser: Jackson 3 API (properties(), JacksonException) combined with the facet-path tracking from apache#175. - JsonUtils, IndexingServiceIntegrationTest and the new JsonResponseParserTest: Jackson 3 (tools.jackson) instead of com.fasterxml; MarkdownDocumentCreator wired into the indexing test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQhKFCmP75K71UV7CiYTqB Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
… build The coc2026 image, native images and JAR are now built from the Spring Boot 4.1 / Spring AI 2.0 line (apache#23), which carries everything on main, so the card describes what that build does: - Fifteen tools, with the three alias tools in the toolbox table. - Step 3: CSV, XML and Markdown samples come from main's src/test/resources. CSV and XML go to Solr's own handlers, which report the payload accepted instead of a count. XML is Solr update XML, so its field names match JSON and the show_ prefix notes go; the Option B XML paste is converted to <add><doc> form. Markdown is one multi-record file and one tool call; its description is in content, which the step 6 note now says. - Step 4: one outcome, an empty facet (apache#175 is in the image). - Native Docker images are prebuilt for amd64 and arm64. - Observability: OTLP/HTTP on 4318 with a URL per signal, ClassName#method span names, timers in milliseconds, JVM metrics over OTLP rather than /actuator/prometheus. - Security: protected resource metadata for /mcp is served at the RFC 9728 path-suffixed URL. - Sizes (52 MB JAR, ~170 MB image) and the verified-against footer. - Table cards: fifteen tools, and ChatGPT in the client list. Every step was replayed against the published image for all four formats, including the Option B pastes, the observability panel against grafana/otel-lgtm 0.33.0, and the security panel against Keycloak. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Aditya Parikh <aditya.m.parikh@gmail.com>
JsonResponseParserclassifies JSON arrays purely by shape: an array ofalternating
[String, non-String, ...]pairs becomes aNamedList, anythingelse becomes a
List. An empty array has no shape to inspect, so a facet on afield that matched zero documents is converted to an empty
List.SolrJ's
QueryResponsecasts everyfacet_counts/facet_fieldsentry toNamedList, so that produces aClassCastExceptionwhenever a faceted fieldhas no matches — a plausible query, not an edge case:
Fix
Give the traversal positional context instead of guessing: thread the node path
through
toNamedList/convertValue, and treat anything directly underfacet_counts/facet_fieldsas aNamedListregardless of shape. The existingshape heuristic still covers the other flat-NamedList sites.
Tests
Adds
JsonResponseParserTest(5 cases), covering populated facets, the emptyfacet, empty/plain arrays outside
facet_fields(which must stayList),and an end-to-end case that feeds the parsed response into a real
QueryResponse.Verified against
main: reverting only theJsonResponseParserchange whilekeeping the new test fails 2 of the 5 cases, one of them with the
ClassCastExceptionabove. With the fix, all 5 pass alongside the existingJsonResponseParserContentTypesTest../gradlew spotlessCheck testis green.🤖 Generated with Claude Code