Skip to content

Commit

Permalink
SOLR-9972: SpellCheckComponent collations and suggestions returned as…
Browse files Browse the repository at this point in the history 
… a JSON object rather than a list

(Christine Poerschke in response to bug report from Ricky Oktavianus Lazuardy)
  • Loading branch information
@cpoerschke
cpoerschke committed Jan 26, 2017
1 parent cf943c5 commit 0187838
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 50 deletions.
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ Other Changes
----------------------
* SOLR-9980: Expose configVersion in core admin status (Jessica Cheng Mallet via Tomás Fernández Löbbe)

* SOLR-9972: SpellCheckComponent collations and suggestions returned as a JSON object rather than a list
(Christine Poerschke in response to bug report from Ricky Oktavianus Lazuardy)

================== 6.4.1 ==================

Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
Expand Down
12 changes: 5 additions & 7 deletions solr/core/src/java/org/apache/solr/handler/component/SpellCheckComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,7 @@ public void process(ResponseBuilder rb) throws IOException {
boolean isCorrectlySpelled = hits > (maxResultsForSuggest==null ? 0 : maxResultsForSuggest);

NamedList response = new SimpleOrderedMap();
NamedList suggestions = toNamedList(shardRequest, spellingResult, q, extendedResults);
response.add("suggestions", suggestions);
response.add("suggestions", toNamedList(shardRequest, spellingResult, q, extendedResults));

if (extendedResults) {
response.add("correctlySpelled", isCorrectlySpelled);
Expand Down Expand Up @@ -300,7 +299,7 @@ protected void addCollationsToResponse(SolrParams params, SpellingResult spellin
//even in cases when the internal rank is the same.
Collections.sort(collations);

NamedList collationList = new NamedList();
NamedList collationList = new SimpleOrderedMap();
for (SpellCheckCollation collation : collations) {
if (collationExtendedResults) {
NamedList extendedResult = new SimpleOrderedMap();
Expand Down Expand Up @@ -424,8 +423,7 @@ public void finishStage(ResponseBuilder rb) {

NamedList response = new SimpleOrderedMap();

NamedList suggestions = toNamedList(false, result, origQuery, extendedResults);
response.add("suggestions", suggestions);
response.add("suggestions", toNamedList(false, result, origQuery, extendedResults));

if (extendedResults) {
response.add("correctlySpelled", isCorrectlySpelled);
Expand All @@ -436,7 +434,7 @@ public void finishStage(ResponseBuilder rb) {
.toArray(new SpellCheckCollation[mergeData.collations.size()]);
Arrays.sort(sortedCollations);

NamedList collations = new NamedList();
NamedList collations = new SimpleOrderedMap();
int i = 0;
while (i < maxCollations && i < sortedCollations.length) {
SpellCheckCollation collation = sortedCollations[i];
Expand Down Expand Up @@ -636,7 +634,7 @@ public SolrSpellChecker getSpellChecker(String name) {

protected NamedList toNamedList(boolean shardRequest,
SpellingResult spellingResult, String origQuery, boolean extendedResults) {
NamedList result = new NamedList();
NamedList result = new SimpleOrderedMap();
Map<Token,LinkedHashMap<String,Integer>> suggestions = spellingResult
.getSuggestions();
boolean hasFreqInfo = spellingResult.hasTokenFrequencyInfo();
Expand Down
79 changes: 36 additions & 43 deletions solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,77 +82,62 @@ public void tearDown() throws Exception {
public void testMaximumResultsForSuggest() throws Exception {
assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, "7")
,"/spellcheck/suggestions/[0]=='brwn'"
,"/spellcheck/suggestions/[1]/numFound==1"
,"/spellcheck/suggestions/brwn/numFound==1"
);
try {
assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, "6")
,"/spellcheck/suggestions/[1]/numFound==1"
);
fail("there should have been no suggestions (6<7)");
} catch(Exception e) {
//correctly threw exception
}

assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, "6")
,"/spellcheck/suggestions=={}");
// there should have been no suggestions (6<7)

assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
"fq", "id:[0 TO 9]", /*returns 10, less selective */ "fq", "lowerfilt:th*", /* returns 8, most selective */
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, ".90")
,"/spellcheck/suggestions/[0]=='brwn'"
,"/spellcheck/suggestions/[1]/numFound==1"
,"/spellcheck/suggestions/brwn/numFound==1"
);
try {
assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
"fq", "id:[0 TO 9]", /*returns 10, less selective */ "fq", "lowerfilt:th*", /* returns 8, most selective */
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, ".80")
,"/spellcheck/suggestions/[1]/numFound==1"
);
fail("there should have been no suggestions ((.8 * 8)<7)");
} catch(Exception e) {
//correctly threw exception
}

assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
"fq", "id:[0 TO 9]", /*returns 10, less selective */ "fq", "lowerfilt:th*", /* returns 8, most selective */
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, ".80")
,"/spellcheck/suggestions=={}");
// there should have been no suggestions ((.8 * 8)<7)


assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
"fq", "id:[0 TO 9]", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST_FQ, "id:[0 TO 9]",
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, ".70")
,"/spellcheck/suggestions/[0]=='brwn'"
,"/spellcheck/suggestions/[1]/numFound==1"
,"/spellcheck/suggestions/brwn/numFound==1"
);
try {
assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
"fq", "id:[0 TO 9]", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST_FQ, "lowerfilt:th*",
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, ".64")
,"/spellcheck/suggestions/[1]/numFound==1"
);
fail("there should have been no suggestions ((.64 * 10)<7)");
} catch(Exception e) {
//correctly threw exception
}

assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","lowerfilt:(this OR brwn)",
"fq", "id:[0 TO 9]", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST_FQ, "lowerfilt:th*",
SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false", SpellingParams.SPELLCHECK_MAX_RESULTS_FOR_SUGGEST, ".64")
,"/spellcheck/suggestions=={}");
// there should have been no suggestions ((.64 * 10)<7)
}

@Test
public void testExtendedResultsCount() throws Exception {
assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", SpellingParams.SPELLCHECK_BUILD, "true", "q","bluo", SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"false")
,"/spellcheck/suggestions/[0]=='bluo'"
,"/spellcheck/suggestions/[1]/numFound==5"
,"/spellcheck/suggestions/bluo/numFound==5"
);

assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", "q","bluo", SpellingParams.SPELLCHECK_COUNT,"3", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"true")
,"/spellcheck/suggestions/[1]/suggestion==[{'word':'blud','freq':1}, {'word':'blue','freq':1}, {'word':'blee','freq':1}]"
,"/spellcheck/suggestions/bluo/suggestion==[{'word':'blud','freq':1}, {'word':'blue','freq':1}, {'word':'blee','freq':1}]"
);
}

@Test
public void test() throws Exception {
assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", "q","documemt")
,"/spellcheck=={'suggestions':['documemt',{'numFound':1,'startOffset':0,'endOffset':8,'suggestion':['document']}]}"
,"/spellcheck=={'suggestions':{'documemt':{'numFound':1,'startOffset':0,'endOffset':8,'suggestion':['document']}}}"
);
}

@Test
public void testNumericQuery() throws Exception {
assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", "q","12346")
,"/spellcheck=={'suggestions':['12346',{'numFound':1,'startOffset':0,'endOffset':5,'suggestion':['12345']}]}"
,"/spellcheck=={'suggestions':{'12346':{'numFound':1,'startOffset':0,'endOffset':5,'suggestion':['12345']}}}"
);
}

Expand Down Expand Up @@ -186,13 +171,21 @@ public void testCollate() throws Exception {
@Test
public void testCollateExtendedResultsWithJsonNl() throws Exception {
final String q = "documemtsss broens";
final String jsonNl = "map";
final String jsonNl = (random().nextBoolean() ? "map" : "arrntv");
final boolean collateExtendedResults = random().nextBoolean();
final List<String> testsList = new ArrayList<String>();
if (collateExtendedResults) {
testsList.add("/spellcheck/collations/collation/collationQuery=='document brown'");
testsList.add("/spellcheck/collations/collation/hits==0");
switch (jsonNl) {
case "arrntv":
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/[0]/name=='documemtsss'");
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/[0]/type=='str'");
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/[0]/value=='document'");
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/[1]/name=='broens'");
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/[1]/type=='str'");
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/[1]/value=='brown'");
break;
case "map":
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/documemtsss=='document'");
testsList.add("/spellcheck/collations/collation/misspellingsAndCorrections/broens=='brown'");
Expand Down Expand Up @@ -311,11 +304,11 @@ public void testThresholdTokenFrequency() throws Exception {
//while "document" is present.

assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", "q","documenq", SpellingParams.SPELLCHECK_DICT, "threshold", SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"true")
,"/spellcheck/suggestions/[1]/suggestion==[{'word':'document','freq':2}]"
,"/spellcheck/suggestions/documenq/suggestion==[{'word':'document','freq':2}]"
);

assertJQ(req("qt",rh, SpellCheckComponent.COMPONENT_NAME, "true", "q","documenq", SpellingParams.SPELLCHECK_DICT, "threshold_direct", SpellingParams.SPELLCHECK_COUNT,"5", SpellingParams.SPELLCHECK_EXTENDED_RESULTS,"true")
,"/spellcheck/suggestions/[1]/suggestion==[{'word':'document','freq':2}]"
,"/spellcheck/suggestions/documenq/suggestion==[{'word':'document','freq':2}]"
);

//TODO: how do we make this into a 1-liner using "assertQ()" ???
Expand Down

0 comments on commit 0187838

Please sign in to comment.