From 4444d1fd30e29b96af9f7442638a870e0dfa4f2d Mon Sep 17 00:00:00 2001 From: Vikas Saurabh Date: Wed, 25 Nov 2015 06:25:18 +0530 Subject: [PATCH] OAK-3686 Solr suggestion results should have 1 row per suggestion with appropriate column names The fix and test case adjustment is (almost) same as one done for the lucene counterpart. But, with multiple rows having '/' as path we'd need to use false for unique parameter for path cursor --- .../plugins/index/solr/query/SolrQueryIndex.java | 45 ++++++++++++++-------- .../jackrabbit/oak/jcr/query/SpellcheckTest.java | 29 +++++++------- .../jackrabbit/oak/jcr/query/SuggestTest.java | 32 +++++++-------- 3 files changed, 63 insertions(+), 43 deletions(-) diff --git a/oak-solr-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/query/SolrQueryIndex.java b/oak-solr-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/query/SolrQueryIndex.java index e50eef4..607554b 100644 --- a/oak-solr-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/query/SolrQueryIndex.java +++ b/oak-solr-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/solr/query/SolrQueryIndex.java @@ -347,8 +347,7 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul SpellCheckResponse spellCheckResponse = queryResponse.getSpellCheckResponse(); if (spellCheckResponse != null && spellCheckResponse.getSuggestions() != null && spellCheckResponse.getSuggestions().size() > 0) { - SolrDocument fakeDoc = getSpellChecks(spellCheckResponse, filter); - queue.add(new SolrResultRow("/", 1.0, fakeDoc)); + putSpellChecks(spellCheckResponse, queue, filter); noDocs = true; } @@ -358,8 +357,7 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul if (suggest != null) { Set> suggestEntries = suggest.entrySet(); if (!suggestEntries.isEmpty()) { - SolrDocument fakeDoc = getSuggestions(suggestEntries, filter); - queue.add(new SolrResultRow("/", 1.0, fakeDoc)); + putSuggestions(suggestEntries, queue, filter); noDocs = true; } } @@ -376,8 +374,9 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul }; } - private SolrDocument getSpellChecks(SpellCheckResponse spellCheckResponse, Filter filter) throws SolrServerException { - SolrDocument fakeDoc = new SolrDocument(); + private void putSpellChecks(SpellCheckResponse spellCheckResponse, + final Deque queue, + Filter filter) throws SolrServerException { List suggestions = spellCheckResponse.getSuggestions(); Collection alternatives = new ArrayList(suggestions.size()); for (SpellCheckResponse.Suggestion suggestion : suggestions) { @@ -396,19 +395,18 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul if (results != null && results.getNumFound() > 0) { for (SolrDocument doc : results) { if (filter.isAccessible(String.valueOf(doc.getFieldValue(configuration.getPathField())))) { - fakeDoc.addField(QueryImpl.REP_SPELLCHECK, alternative); + queue.add (new SolrResultRow(alternative)); break; } } } } - - return fakeDoc; } - private SolrDocument getSuggestions(Set> suggestEntries, Filter filter) throws SolrServerException { + private void putSuggestions(Set> suggestEntries, + final Deque queue, + Filter filter) throws SolrServerException { Collection> retrievedSuggestions = new HashSet>(); - SolrDocument fakeDoc = new SolrDocument(); for (Map.Entry suggester : suggestEntries) { SimpleOrderedMap suggestionResponses = ((SimpleOrderedMap) suggester.getValue()); for (Map.Entry suggestionResponse : suggestionResponses) { @@ -438,13 +436,13 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul if (results != null && results.getNumFound() > 0) { for (SolrDocument doc : results) { if (filter.isAccessible(String.valueOf(doc.getFieldValue(configuration.getPathField())))) { - fakeDoc.addField(QueryImpl.REP_SUGGEST, "{term=" + suggestion.get("term") + ",weight=" + suggestion.get("weight") + "}"); + queue.add (new SolrResultRow(suggestion.get("term").toString(), + Double.parseDouble(suggestion.get("weight").toString()))); break; } } } } - return fakeDoc; } static boolean isIgnoredProperty(String propertyName, OakSolrConfiguration configuration) { @@ -500,11 +498,25 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul final String path; final double score; final SolrDocument doc; + final String suggestion; - SolrResultRow(String path, double score, SolrDocument doc) { + private SolrResultRow(String path, double score, SolrDocument doc, String suggestion) { this.path = path; this.score = score; this.doc = doc; + this.suggestion = suggestion; + } + + SolrResultRow(String path, double score, SolrDocument doc) { + this (path, score, doc, null); + } + + SolrResultRow(String suggestion, double score) { + this ("/", score, null, suggestion); + } + + SolrResultRow(String suggestion) { + this ("/", 1.0, null, suggestion); } @Override @@ -546,7 +558,7 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul }; this.plan = plan; - this.pathCursor = new Cursors.PathCursor(pathIterator, true, settings); + this.pathCursor = new Cursors.PathCursor(pathIterator, false, settings); } @@ -581,6 +593,9 @@ public class SolrQueryIndex implements FulltextQueryIndex, QueryIndex.AdvanceFul if (QueryImpl.JCR_SCORE.equals(columnName)) { return PropertyValues.newDouble(currentRow.score); } + if (QueryImpl.REP_SPELLCHECK.equals(columnName) || QueryImpl.REP_SUGGEST.equals(columnName)) { + return PropertyValues.newString(currentRow.suggestion); + } Collection fieldValues = currentRow.doc.getFieldValues(columnName); String value; if (fieldValues != null && fieldValues.size() > 0) { diff --git a/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SpellcheckTest.java b/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SpellcheckTest.java index af47b8d..54a38c2 100644 --- a/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SpellcheckTest.java +++ b/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SpellcheckTest.java @@ -27,8 +27,11 @@ import javax.jcr.query.QueryResult; import javax.jcr.query.Row; import javax.jcr.query.RowIterator; +import com.google.common.collect.Lists; import org.apache.jackrabbit.core.query.AbstractQueryTest; +import java.util.List; + /** * Tests the spellcheck support. */ @@ -45,9 +48,10 @@ public class SpellcheckTest extends AbstractQueryTest { String sql = "SELECT [rep:spellcheck()] FROM nt:base WHERE [jcr:path] = '/' AND SPELLCHECK('helo')"; Query q = qm.createQuery(sql, Query.SQL); - String result = getResult(q.execute(), "rep:spellcheck()"); + List result = getResult(q.execute(), "rep:spellcheck()"); assertNotNull(result); - assertEquals("[hello, hold]", result); + assertEquals(2, result.size()); + assertEquals("[hello, hold]", result.toString()); } public void testSpellcheckXPath() throws Exception { @@ -61,9 +65,10 @@ public class SpellcheckTest extends AbstractQueryTest { String xpath = "/jcr:root[rep:spellcheck('helo')]/(rep:spellcheck())"; Query q = qm.createQuery(xpath, Query.XPATH); - String result = getResult(q.execute(), "rep:spellcheck()"); + List result = getResult(q.execute(), "rep:spellcheck()"); assertNotNull(result); - assertEquals("[hello, hold]", result); + assertEquals(2, result.size()); + assertEquals("[hello, hold]", result.toString()); } public void testSpellcheckMultipleWords() throws Exception { @@ -81,22 +86,20 @@ public class SpellcheckTest extends AbstractQueryTest { String xpath = "/jcr:root[rep:spellcheck('votin in ontari')]/(rep:spellcheck())"; Query q = qm.createQuery(xpath, Query.XPATH); - String result = getResult(q.execute(), "rep:spellcheck()"); + List result = getResult(q.execute(), "rep:spellcheck()"); assertNotNull(result); - assertEquals("voting in ontario", result); + assertEquals(1, result.size()); + assertEquals("voting in ontario", result.get(0)); } - static String getResult(QueryResult result, String propertyName) throws RepositoryException { - StringBuilder buff = new StringBuilder(); + static List getResult(QueryResult result, String propertyName) throws RepositoryException { + List results = Lists.newArrayList(); RowIterator it = result.getRows(); while (it.hasNext()) { - if (buff.length() > 0) { - buff.append(", "); - } Row row = it.nextRow(); - buff.append(row.getValue(propertyName).getString()); + results.add(row.getValue(propertyName).getString()); } - return buff.toString(); + return results; } } \ No newline at end of file diff --git a/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SuggestTest.java b/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SuggestTest.java index 3bd2069..1e6e9fc 100644 --- a/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SuggestTest.java +++ b/oak-solr-core/src/test/java/org/apache/jackrabbit/oak/jcr/query/SuggestTest.java @@ -27,8 +27,11 @@ import javax.jcr.query.QueryResult; import javax.jcr.query.Row; import javax.jcr.query.RowIterator; +import com.google.common.collect.Lists; import org.apache.jackrabbit.core.query.AbstractQueryTest; +import java.util.List; + /** * Tests the suggest support. */ @@ -45,10 +48,11 @@ public class SuggestTest extends AbstractQueryTest { String sql = "SELECT [rep:suggest()] FROM nt:base WHERE [jcr:path] = '/' AND SUGGEST('in 201')"; Query q = qm.createQuery(sql, Query.SQL); - String result = getResult(q.execute(), "rep:suggest()"); + List result = getResult(q.execute(), "rep:suggest()"); assertNotNull(result); - assertEquals("[{term=in 2015 a red fox is still a fox,weight=1}, " + - "{term=in 2015 my fox is red, like mike's fox and john's fox,weight=1}]", result); + assertEquals(2, result.size()); + assertTrue(result.contains("in 2015 a red fox is still a fox")); + assertTrue(result.contains("in 2015 my fox is red, like mike's fox and john's fox")); } public void testSuggestXPath() throws Exception { @@ -62,10 +66,11 @@ public class SuggestTest extends AbstractQueryTest { String xpath = "/jcr:root[rep:suggest('in 201')]/(rep:suggest())"; Query q = qm.createQuery(xpath, Query.XPATH); - String result = getResult(q.execute(), "rep:suggest()"); + List result = getResult(q.execute(), "rep:suggest()"); assertNotNull(result); - assertEquals("[{term=in 2015 a red fox is still a fox,weight=1}, " + - "{term=in 2015 my fox is red, like mike's fox and john's fox,weight=1}]", result); + assertEquals(2, result.size()); + assertTrue(result.contains("in 2015 a red fox is still a fox")); + assertTrue(result.contains("in 2015 my fox is red, like mike's fox and john's fox")); } public void testNoSuggestions() throws Exception { @@ -79,22 +84,19 @@ public class SuggestTest extends AbstractQueryTest { String sql = "SELECT [rep:suggest()] FROM nt:base WHERE [jcr:path] = '/' AND SUGGEST('blablabla')"; Query q = qm.createQuery(sql, Query.SQL); - String result = getResult(q.execute(), "rep:suggest()"); + List result = getResult(q.execute(), "rep:suggest()"); assertNotNull(result); - assertEquals("[]", result); + assertEquals("There shouldn't be any suggestions", 0, result.size()); } - static String getResult(QueryResult result, String propertyName) throws RepositoryException { - StringBuilder buff = new StringBuilder(); + static List getResult(QueryResult result, String propertyName) throws RepositoryException { + List results = Lists.newArrayList(); RowIterator it = result.getRows(); while (it.hasNext()) { - if (buff.length() > 0) { - buff.append(", "); - } Row row = it.nextRow(); - buff.append(row.getValue(propertyName).getString()); + results.add(row.getValue(propertyName).getString()); } - return buff.toString(); + return results; } } \ No newline at end of file -- 2.5.3