SOLR-17316: make SolrJ response objects work with non-binary ResponseParsers#4640
Open
serhiy-bzhezytskyy wants to merge 2 commits into
Open
Conversation
…arsers 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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/SOLR-17316
SolrJ's response classes assume binary-parser types, so reading a response parsed by a non-binary parser (the JSON parser) throws ClassCastException. Two commits so the small safe fix can go in on its own if you'd rather.
Commit 1 is the narrow fix: getStatus/getQTime cast to Integer, which CCEs when JSON gives Long. Widened via Number. That's it — resolves the getters the issue names.
Commit 2 goes further. It's not just the header — getResults(), facets, grouping all break under JSON too, because the parser hands back raw Map/List where the code wants NamedList/SolrDocumentList. So there's a ResponseNormalizer that converts a parsed response to the canonical shape at the client boundary (no-op for binary/XML), gated by a new ResponseParser.producesCanonicalForm() so only the JSON map parser triggers it. Both transports covered, binary pays nothing. Plus the remaining Integer/Float casts widened.
Heads up: commit 2 basically does what SOLR-3451 asked for in 2012, which was closed Won't Fix ("solr does not have a way to write a JSON response and read the same value"). Still true for json.nl=flat since it's lossy, but json.nl=map round-trips and the normalizer does the rest. So I'd treat commit 2 as reopening that discussion — fine to split it out or take it to dev@ if you'd prefer, commit 1 stands alone either way.
Tests: the normalizer + edge cases, binary/xml/json-map parity, the affected sections (grouping, facets, stats, spellcheck, highlighting, terms, moreLikeThis, analysis, Luke, schema), and an end-to-end JSON query on both the Jetty and JDK clients.