Solr
  1. Solr
  2. SOLR-6709

ClassCastException in QueryResponse after applying XMLResponseParser on a response containing an "expanded" section

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.2, 6.0
    • Component/s: SolrJ
    • Labels:
      None

      Description

      Shouldn't the following code work on the attached input file?
      It matches the structure of a Solr response with wt=xml.

      import java.io.InputStream;
      import org.apache.solr.client.solrj.ResponseParser;
      import org.apache.solr.client.solrj.impl.XMLResponseParser;
      import org.apache.solr.client.solrj.response.QueryResponse;
      import org.apache.solr.common.util.NamedList;
      import org.junit.Test;
      
      public class ParseXmlExpandedTest {
      	@Test
      	public void test() {
      		ResponseParser responseParser = new XMLResponseParser();
      		InputStream inStream = getClass()
      				.getResourceAsStream("test-response.xml");
      		NamedList<Object> response = responseParser
      				.processResponse(inStream, "UTF-8");
      		QueryResponse queryResponse = new QueryResponse(response, null);
      	}
      }

      Unexpectedly (for me), it throws a
      java.lang.ClassCastException: org.apache.solr.common.util.SimpleOrderedMap cannot be cast to java.util.Map
      at org.apache.solr.client.solrj.response.QueryResponse.setResponse(QueryResponse.java:126)

      Am I missing something, is XMLResponseParser deprecated or something?

      We use a setup like this to "mock" a QueryResponse for unit tests in our service that post-processes the Solr response.
      Obviously, it works with the javabin format which SolrJ uses internally.
      But that is no appropriate format for unit tests, where the response should be human readable.

      I think there's some conversion missing in QueryResponse or XMLResponseParser.

      Note: The null value supplied as SolrServer argument to the constructor of QueryResponse shouldn't have an effect as the error occurs before the parameter is even used.

      1. SOLR-6709.patch
        11 kB
        Varun Thacker
      2. SOLR-6709.patch
        11 kB
        Varun Thacker
      3. SOLR-6709.patch
        8 kB
        Varun Thacker
      4. test-response.xml
        0.3 kB
        Simon Endele

        Activity

        Hide
        Varun Thacker added a comment -

        Patch with fix and test case.

        testExpandComponent will fail if the entire patch is not applied.

        Joel Bernstein Would be great if you could have a look at it.

        Show
        Varun Thacker added a comment - Patch with fix and test case. testExpandComponent will fail if the entire patch is not applied. Joel Bernstein Would be great if you could have a look at it.
        Hide
        Joel Bernstein added a comment -

        I should have a chance to take a look at this this week.

        Show
        Joel Bernstein added a comment - I should have a chance to take a look at this this week.
        Hide
        Varun Thacker added a comment -

        The patch I had uploaded was failing with the expand components distrib tests. I realized that it was because the group keys that were being put into the HashMap the ordering when retrieving it was the same. Just changing Map<String, DocSlice> outMap = new HashMap<>(); to Map<String, DocSlice> outMap = new LinkedHashMap<>(); will cause the test to fail.

        I'll work on a new patch

        Show
        Varun Thacker added a comment - The patch I had uploaded was failing with the expand components distrib tests. I realized that it was because the group keys that were being put into the HashMap the ordering when retrieving it was the same. Just changing Map<String, DocSlice> outMap = new HashMap<>(); to Map<String, DocSlice> outMap = new LinkedHashMap<>(); will cause the test to fail. I'll work on a new patch
        Hide
        Varun Thacker added a comment -

        New patch. Works fine in distirb mode also.

        One question though - Was the ordering of the group keys guaranteed?

        Show
        Varun Thacker added a comment - New patch. Works fine in distirb mode also. One question though - Was the ordering of the group keys guaranteed?
        Hide
        Varun Thacker added a comment -

        Hi Joel,

        I would really appreciate a review on the patch. Not sure what the contract is on the ordering of group keys.

        Show
        Varun Thacker added a comment - Hi Joel, I would really appreciate a review on the patch. Not sure what the contract is on the ordering of group keys.
        Hide
        Joel Bernstein added a comment -

        Hi Varun,

        There was no contract on the ordering of group keys. The groups are meant to be read into a Map and then accessed by key.

        Show
        Joel Bernstein added a comment - Hi Varun, There was no contract on the ordering of group keys. The groups are meant to be read into a Map and then accessed by key.
        Hide
        Varun Thacker added a comment -

        Thanks Joel for clarifying that.

        I'll run the tests and commit it shortly then.

        Show
        Varun Thacker added a comment - Thanks Joel for clarifying that. I'll run the tests and commit it shortly then.
        Hide
        Joel Bernstein added a comment -

        Just reviewed the test case and all the groups are accessed directly by name, this simulates access into a map. The order of the documents within the groups matter in the test case. But the order of the groups themselves does not.

        The only thing I worry about is that someone might imply that order matters if we use an ordered map, but we could document this more closely.

        Show
        Joel Bernstein added a comment - Just reviewed the test case and all the groups are accessed directly by name, this simulates access into a map. The order of the documents within the groups matter in the test case. But the order of the groups themselves does not. The only thing I worry about is that someone might imply that order matters if we use an ordered map, but we could document this more closely.
        Hide
        Varun Thacker added a comment -

        Added javadocs to

        {QueryResult.getExpandedResults}

        . Is there anything else that needs to be addressed?

        Show
        Varun Thacker added a comment - Added javadocs to {QueryResult.getExpandedResults} . Is there anything else that needs to be addressed?
        Hide
        Joel Bernstein added a comment -

        Looking at the patch, I don't see any issues. As long as the tests are passing I think we are fine.

        Show
        Joel Bernstein added a comment - Looking at the patch, I don't see any issues. As long as the tests are passing I think we are fine.
        Hide
        Varun Thacker added a comment -

        Forgot to append the Jira number. Here is the revision where it was committed to trunk - https://svn.apache.org/viewvc?view=revision&revision=1670569

        Show
        Varun Thacker added a comment - Forgot to append the Jira number. Here is the revision where it was committed to trunk - https://svn.apache.org/viewvc?view=revision&revision=1670569
        Hide
        ASF subversion and git services added a comment -

        Commit 1670579 from Varun Thacker in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1670579 ]

        SOLR-6709: Fix QueryResponse to deal with the expanded section when using the XMLResponseParser (merged from trunk r1670569)

        Show
        ASF subversion and git services added a comment - Commit 1670579 from Varun Thacker in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1670579 ] SOLR-6709 : Fix QueryResponse to deal with the expanded section when using the XMLResponseParser (merged from trunk r1670569)
        Hide
        Varun Thacker added a comment -

        Thank you Simon for reporting and Joel for reviewing the patch.

        Show
        Varun Thacker added a comment - Thank you Simon for reporting and Joel for reviewing the patch.
        Hide
        Simon Endele added a comment -

        Thank you guys very much for fixing/reviewing and happy Easter!

        Show
        Simon Endele added a comment - Thank you guys very much for fixing/reviewing and happy Easter!
        Hide
        Anshum Gupta added a comment -

        Bulk close for 5.2.0.

        Show
        Anshum Gupta added a comment - Bulk close for 5.2.0.

          People

          • Assignee:
            Varun Thacker
            Reporter:
            Simon Endele
          • Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development