Solr
  1. Solr
  2. SOLR-4702

Velocity templates not rendering spellcheck suggestions correctly

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.2
    • Fix Version/s: 4.3, 4.10, 6.0
    • Component/s: contrib - Velocity
    • Labels:
      None

      Description

      The spellcheck links, AKA "Did you mean", aren't rendered correctly.

      Instead of just having the corrected words, they have some .toString gibberish because the object being serialized is too high up in the tree.

      This breaks both the link text displayed to the user, and the href used for the anchor tag.

      Example:
      Search for "electronicss OR monitor" and you get:
      Did you mean {collationQuery=electronics OR monitor,hits=14,misspellingsAndCorrections={electronicss=electronics,monitor=monitor}}?

      But you should just see:
      Did you mean electronics OR monitor? (with hyperlinked "electronics OR monitor")

      The actual query submitted by those links is similarly broken. Possibly the templates were developed before collation was added and/or enabled by default.

      To see this bug at all with the example configs and docs you'll need to have applied SOLR-4680 or SOLR-4681 against 4.2 or trunk.

      1. SOLR-4702.patch
        2 kB
        Erik Hatcher
      2. SOLR-4702.patch
        1 kB
        Erik Hatcher
      3. SOLR-4702.patch
        2 kB
        Mark Bennett

        Issue Links

          Activity

          Hide
          Mark Bennett added a comment -

          Fixes "Did you mean" spellcheck logic to use proper collation elements.

          Also breaks out that logic into a new template did_you_mean.vm, and reformats the code to make it a bit easier to read.

          Show
          Mark Bennett added a comment - Fixes "Did you mean" spellcheck logic to use proper collation elements. Also breaks out that logic into a new template did_you_mean.vm, and reformats the code to make it a bit easier to read.
          Hide
          Erik Hatcher added a comment -

          Mark - here's an updated patch that simplifies a bit of the Velocity stuff, hopefully without breaking anything.

          Show
          Erik Hatcher added a comment - Mark - here's an updated patch that simplifies a bit of the Velocity stuff, hopefully without breaking anything.
          Hide
          Erik Hatcher added a comment -

          Ideally we'd have tests that check that the example app works where we could codify expectations from the example app to make sure things like this don't break as components/internals change. Thoughts on ways to go about that? Selenium tests? Or somehow do this with JUnit? Do we still have tests that drive from the example configuration?

          Show
          Erik Hatcher added a comment - Ideally we'd have tests that check that the example app works where we could codify expectations from the example app to make sure things like this don't break as components/internals change. Thoughts on ways to go about that? Selenium tests? Or somehow do this with JUnit? Do we still have tests that drive from the example configuration?
          Hide
          Erik Hatcher added a comment -

          Here's an update that fixes the reference to the no longer used textSpell field type and uses text_general on the text field instead. If the queryAnalyzerFieldType specified field type doesn't exist, it uses the WhitespaceAnalyzer so it still worked without an error.

          Show
          Erik Hatcher added a comment - Here's an update that fixes the reference to the no longer used textSpell field type and uses text_general on the text field instead. If the queryAnalyzerFieldType specified field type doesn't exist, it uses the WhitespaceAnalyzer so it still worked without an error.
          Hide
          Erik Hatcher added a comment -

          Committed this to both 4x (r1467413) and trunk (r1467416).

          Mark - let me know if there's anything else needed here.

          Show
          Erik Hatcher added a comment - Committed this to both 4x (r1467413) and trunk (r1467416). Mark - let me know if there's anything else needed here.
          Hide
          Mark Bennett added a comment -

          Hi Erik,

          Thanks.

          Two options we might consider:
          1: I didn't see a for loop, and in some cases there could be multiple collations?
          2: When debugging, seeing the raw suggestions, prior to collation filtering, can be nice. Though I'm not sure if that's a good fit for these templates or not. If we come back and add more options, maybe a "showRawSpellingSuggestions=true|FALSE" might be nice.

          Show
          Mark Bennett added a comment - Hi Erik, Thanks. Two options we might consider: 1: I didn't see a for loop, and in some cases there could be multiple collations? 2: When debugging, seeing the raw suggestions, prior to collation filtering, can be nice. Though I'm not sure if that's a good fit for these templates or not. If we come back and add more options, maybe a "showRawSpellingSuggestions=true|FALSE" might be nice.
          Hide
          ASF subversion and git services added a comment -

          Commit 1616073 from Erik Hatcher in branch 'dev/trunk'
          [ https://svn.apache.org/r1616073 ]

          SOLR-4702: Added support for multiple spellcheck collations to /browse UI.

          Show
          ASF subversion and git services added a comment - Commit 1616073 from Erik Hatcher in branch 'dev/trunk' [ https://svn.apache.org/r1616073 ] SOLR-4702 : Added support for multiple spellcheck collations to /browse UI.
          Hide
          ASF subversion and git services added a comment -

          Commit 1616074 from Erik Hatcher in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1616074 ]

          SOLR-4702: Added support for multiple spellcheck collations to /browse UI.

          Show
          ASF subversion and git services added a comment - Commit 1616074 from Erik Hatcher in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1616074 ] SOLR-4702 : Added support for multiple spellcheck collations to /browse UI.
          Hide
          Erik Hatcher added a comment - - edited

          Mark Bennett - apologies for the delay on this. The fix was committed earlier (for Solr 4.3), and I left this open to (eventually, heh) address your feedback after the commit. I just committed an enhancement that shows all possible collation queries, along with their hit count; note that the counts are within the currently selected filters not globally. Regarding the debugging, I recommend simply adding &wt=xml (or clicking the link in the footer) to get at the raw response suitable for developer-level debugging. I also often simply put into the template $response... and $object.class kind of expressions directly into the template and refresh my browser to see gory details that help craft the templates. We certainly could add more insight into the UI as you suggest, though under a new Jira.

          Show
          Erik Hatcher added a comment - - edited Mark Bennett - apologies for the delay on this. The fix was committed earlier (for Solr 4.3), and I left this open to (eventually, heh) address your feedback after the commit. I just committed an enhancement that shows all possible collation queries, along with their hit count; note that the counts are within the currently selected filters not globally. Regarding the debugging, I recommend simply adding &wt=xml (or clicking the link in the footer) to get at the raw response suitable for developer-level debugging. I also often simply put into the template $response... and $object.class kind of expressions directly into the template and refresh my browser to see gory details that help craft the templates. We certainly could add more insight into the UI as you suggest, though under a new Jira.

            People

            • Assignee:
              Erik Hatcher
              Reporter:
              Mark Bennett
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development