Suggest: Add parsing from xContent to PhraseSuggestionBuilder

For the ongoing search refactoring (elastic#10217) the PhraseSuggestionBuilder gets a way of parsing from xContent that will eventually replace the current SuggestParseElement. This PR adds the fromXContent method to the PhraseSuggestionBuilder and also adds parsing code for the common suggestion parameters to SuggestionBuilder. Also adding links from the Suggester implementations registeres in the Suggesters registry to the corresponding prototype that is going to be used for parsing once the refactoring is done and we switch from parsing on shard to parsing on coordinating node.
cbuescher · Feb 8, 2016 · 2ae6420 · 2ae6420
1 parent 2dec129
commit 2ae6420
Show file tree

Hide file tree

Showing 19 changed files with 483 additions and 140 deletions.
diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml
@@ -916,7 +916,6 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]SuggestUtils.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]Suggesters.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]completion[/\\]CompletionSuggestParser.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]completion[/\\]CompletionSuggester.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]completion[/\\]context[/\\]CategoryContextMapping.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]completion[/\\]context[/\\]ContextMapping.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]completion[/\\]context[/\\]GeoContextMapping.java" checks="LineLength" />
@@ -927,12 +926,9 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]LinearInterpoatingScorer.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]NoisyChannelSpellChecker.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]PhraseSuggestParser.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]PhraseSuggester.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]PhraseSuggestionBuilder.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]StupidBackoffScorer.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]WordScorer.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]term[/\\]TermSuggestParser.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]term[/\\]TermSuggester.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]snapshots[/\\]RestoreService.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]snapshots[/\\]SnapshotInfo.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]snapshots[/\\]SnapshotShardFailure.java" checks="LineLength" />
@@ -1446,7 +1442,6 @@
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]query[/\\]ExistsIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]query[/\\]MultiMatchQueryIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]query[/\\]SearchQueryIT.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]rescore[/\\]QueryRescoreBuilderTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]scroll[/\\]DuelScrollIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]scroll[/\\]SearchScrollIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]scroll[/\\]SearchScrollWithFailingNodesIT.java" checks="LineLength" />
@@ -1462,7 +1457,6 @@
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]completion[/\\]GeoContextMappingTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]DirectCandidateGeneratorTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]NoisyChannelSpellCheckerTests.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]search[/\\]suggest[/\\]phrase[/\\]SmoothingModelTestCase.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]similarity[/\\]SimilarityIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]snapshots[/\\]AbstractSnapshotIntegTestCase.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]snapshots[/\\]BlobStoreFormatIT.java" checks="LineLength" />

diff --git a/core/src/main/java/org/elasticsearch/search/suggest/SuggestParseElement.java b/core/src/main/java/org/elasticsearch/search/suggest/SuggestParseElement.java
@@ -123,7 +123,6 @@ public SuggestionSearchContext parseInternal(XContentParser parser, MapperServic
             SuggestUtils.verifySuggestion(mapperService, globalText, suggestionContext);
             suggestionSearchContext.addSuggestion(suggestionName, suggestionContext);
         }
-
         return suggestionSearchContext;
     }
 }
diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java
@@ -29,8 +29,18 @@ public abstract class Suggester<T extends SuggestionSearchContext.SuggestionCont
     protected abstract Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>>
         innerExecute(String name, T suggestion, IndexSearcher searcher, CharsRefBuilder spare) throws IOException;
 
+    /**
+     * link the suggester to its corresponding {@link SuggestContextParser}
+     * TODO: This method should eventually be removed by {@link #getBuilderPrototype()} once
+     * we don't directly parse from xContent to the SuggestionContext any more
+     */
     public abstract SuggestContextParser getContextParser();
 
+    /**
+     * link the suggester to its corresponding {@link SuggestionBuilder}
+     */
+    public abstract SuggestionBuilder<?> getBuilderPrototype();
+
     public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>>
         execute(String name, T suggestion, IndexSearcher searcher, CharsRefBuilder spare) throws IOException {
         // #3469 We want to ignore empty shards

diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java
@@ -64,4 +64,16 @@ private static Map<String, Suggester> addBuildIns(Map<String, Suggester> suggest
     public Suggester get(String type) {
         return parsers.get(type);
     }
+
+    public SuggestionBuilder<?> getSuggestionPrototype(String suggesterName) {
+        Suggester<?> suggester = parsers.get(suggesterName);
+        if (suggester == null) {
+            throw new IllegalArgumentException("suggester with name [" + suggesterName + "] not supported");
+        }
+        SuggestionBuilder<?> suggestParser = suggester.getBuilderPrototype();
+        if (suggestParser == null) {
+            throw new IllegalArgumentException("suggester with name [" + suggesterName + "] not supported");
+        }
+        return suggestParser;
+    }
 }
diff --git a/core/src/main/java/org/elasticsearch/search/suggest/SuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/SuggestionBuilder.java
@@ -21,10 +21,13 @@
 
 import org.elasticsearch.action.support.ToXContentToBytes;
 import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.io.stream.NamedWriteable;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.index.query.QueryParseContext;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -138,12 +141,62 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         return builder;
     }
 
+    protected abstract XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException;
+
+    public static SuggestionBuilder<?> fromXContent(QueryParseContext parseContext, String suggestionName, Suggesters suggesters)
+            throws IOException {
+        XContentParser parser = parseContext.parser();
+        ParseFieldMatcher parsefieldMatcher = parseContext.parseFieldMatcher();
+        XContentParser.Token token;
+        String fieldName = null;
+        String suggestText = null;
+        String prefix = null;
+        String regex = null;
+        SuggestionBuilder<?> suggestionBuilder = null;
+
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+            if (token == XContentParser.Token.FIELD_NAME) {
+                fieldName = parser.currentName();
+            } else if (token.isValue()) {
+                if (parsefieldMatcher.match(fieldName, TEXT_FIELD)) {
+                    suggestText = parser.text();
+                } else if (parsefieldMatcher.match(fieldName, PREFIX_FIELD)) {
+                    prefix = parser.text();
+                } else if (parsefieldMatcher.match(fieldName, REGEX_FIELD)) {
+                    regex = parser.text();
+                } else {
+                    throw new IllegalArgumentException("[suggestion] does not support [" + fieldName + "]");
+                }
+            } else if (token == XContentParser.Token.START_OBJECT) {
+                if (suggestionName == null) {
+                    throw new IllegalArgumentException("Suggestion must have name");
+                }
+                SuggestionBuilder<?> suggestParser = suggesters.getSuggestionPrototype(fieldName);
+                if (suggestParser == null) {
+                    throw new IllegalArgumentException("Suggester[" + fieldName + "] not supported");
+                }
+                suggestionBuilder = suggestParser.innerFromXContent(parseContext, suggestionName);
+            }
+        }
+        if (suggestText != null) {
+            suggestionBuilder.text(suggestText);
+        }
+        if (prefix != null) {
+            suggestionBuilder.prefix(prefix);
+        }
+        if (regex != null) {
+            suggestionBuilder.regex(regex);
+        }
+        return suggestionBuilder;
+    }
+
+    protected abstract SuggestionBuilder<T> innerFromXContent(QueryParseContext parseContext, String name) throws IOException;
+
     private String getSuggesterName() {
         //default impl returns the same as writeable name, but we keep the distinction between the two just to make sure
         return getWriteableName();
     }
 
-    protected abstract XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException;
 
     /**
      * Sets from what field to fetch the candidate suggestions from. This is an

diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java
@@ -38,6 +38,7 @@
 import org.elasticsearch.search.suggest.Suggest;
 import org.elasticsearch.search.suggest.SuggestContextParser;
 import org.elasticsearch.search.suggest.Suggester;
+import org.elasticsearch.search.suggest.SuggestionBuilder;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -50,6 +51,7 @@
 
 public class CompletionSuggester extends Suggester<CompletionSuggestionContext> {
 
+    @Override
     public SuggestContextParser getContextParser() {
         return new CompletionSuggestParser(this);
     }
@@ -86,7 +88,8 @@ protected Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Sugges
                 for (String field : payloadFields) {
                     MappedFieldType payloadFieldType = suggestionContext.getMapperService().fullName(field);
                     if (payloadFieldType != null) {
-                        final AtomicFieldData data = suggestionContext.getIndexFieldDataService().getForField(payloadFieldType).load(subReaderContext);
+                        final AtomicFieldData data = suggestionContext.getIndexFieldDataService().getForField(payloadFieldType)
+                                .load(subReaderContext);
                         final ScriptDocValues scriptValues = data.getScriptValues();
                         scriptValues.setNextDocId(subDocId);
                         payload.put(field, new ArrayList<>(scriptValues.getValues()));
@@ -262,4 +265,9 @@ public TopSuggestDocs get() throws IOException {
             }
         }
     }
+
+    @Override
+    public SuggestionBuilder<?> getBuilderPrototype() {
+        return CompletionSuggestionBuilder.PROTOTYPE;
+    }
 }
diff --git a/...rc/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java b/...rc/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java
@@ -27,6 +27,7 @@
 import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.RegexpFlag;
 import org.elasticsearch.search.suggest.SuggestionBuilder;
 import org.elasticsearch.search.suggest.completion.context.CategoryQueryContext;
@@ -50,7 +51,7 @@
 public class CompletionSuggestionBuilder extends SuggestionBuilder<CompletionSuggestionBuilder> {
 
     public static final CompletionSuggestionBuilder PROTOTYPE = new CompletionSuggestionBuilder("_na_"); // name doesn't matter
-    final static String SUGGESTION_NAME = "completion";
+    static final String SUGGESTION_NAME = "completion";
     static final ParseField PAYLOAD_FIELD = new ParseField("payload");
     static final ParseField CONTEXTS_FIELD = new ParseField("contexts", "context");
 
@@ -369,6 +370,11 @@ protected XContentBuilder innerToXContent(XContentBuilder builder, Params params
         return builder;
     }
 
+    @Override
+    protected CompletionSuggestionBuilder innerFromXContent(QueryParseContext parseContext, String name) throws IOException {
+        return new CompletionSuggestionBuilder(name);
+    }
+
     @Override
     public String getWriteableName() {
         return SUGGESTION_NAME;

diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java
@@ -44,6 +44,7 @@
 import org.elasticsearch.search.suggest.SuggestContextParser;
 import org.elasticsearch.search.suggest.SuggestUtils;
 import org.elasticsearch.search.suggest.Suggester;
+import org.elasticsearch.search.suggest.SuggestionBuilder;
 import org.elasticsearch.search.suggest.phrase.NoisyChannelSpellChecker.Result;
 
 import java.io.IOException;
@@ -65,14 +66,14 @@ public PhraseSuggester(ScriptService scriptService, IndicesService indicesServic
     /*
      * More Ideas:
      *   - add ability to find whitespace problems -> we can build a poor mans decompounder with our index based on a automaton?
-     *   - add ability to build different error models maybe based on a confusion matrix?   
+     *   - add ability to build different error models maybe based on a confusion matrix?
      *   - try to combine a token with its subsequent token to find / detect word splits (optional)
      *      - for this to work we need some way to defined the position length of a candidate
      *   - phonetic filters could be interesting here too for candidate selection
      */
     @Override
-    public Suggestion<? extends Entry<? extends Option>> innerExecute(String name, PhraseSuggestionContext suggestion, IndexSearcher searcher,
-            CharsRefBuilder spare) throws IOException {
+    public Suggestion<? extends Entry<? extends Option>> innerExecute(String name, PhraseSuggestionContext suggestion,
+            IndexSearcher searcher, CharsRefBuilder spare) throws IOException {
         double realWordErrorLikelihood = suggestion.realworldErrorLikelyhood();
         final PhraseSuggestion response = new PhraseSuggestion(name, suggestion.getSize());
         final IndexReader indexReader = searcher.getIndexReader();
@@ -84,21 +85,23 @@ public Suggestion<? extends Entry<? extends Option>> innerExecute(String name, P
             DirectSpellChecker directSpellChecker = SuggestUtils.getDirectSpellChecker(generator);
             Terms terms = MultiFields.getTerms(indexReader, generator.field());
             if (terms !=  null) {
-                gens.add(new DirectCandidateGenerator(directSpellChecker, generator.field(), generator.suggestMode(), 
-                        indexReader, realWordErrorLikelihood, generator.size(), generator.preFilter(), generator.postFilter(), terms));    
+                gens.add(new DirectCandidateGenerator(directSpellChecker, generator.field(), generator.suggestMode(),
+                        indexReader, realWordErrorLikelihood, generator.size(), generator.preFilter(), generator.postFilter(), terms));
             }
         }
         final String suggestField = suggestion.getField();
         final Terms suggestTerms = MultiFields.getTerms(indexReader, suggestField);
         if (gens.size() > 0 && suggestTerms != null) {
-            final NoisyChannelSpellChecker checker = new NoisyChannelSpellChecker(realWordErrorLikelihood, suggestion.getRequireUnigram(), suggestion.getTokenLimit());
+            final NoisyChannelSpellChecker checker = new NoisyChannelSpellChecker(realWordErrorLikelihood, suggestion.getRequireUnigram(),
+                    suggestion.getTokenLimit());
             final BytesRef separator = suggestion.separator();
-            WordScorer wordScorer = suggestion.model().newScorer(indexReader, suggestTerms, suggestField, realWordErrorLikelihood, separator);
+            WordScorer wordScorer = suggestion.model().newScorer(indexReader, suggestTerms, suggestField, realWordErrorLikelihood,
+                    separator);
             Result checkerResult;
             try (TokenStream stream = checker.tokenStream(suggestion.getAnalyzer(), suggestion.getText(), spare, suggestion.getField())) {
-                checkerResult = checker.getCorrections(stream, new MultiCandidateGeneratorWrapper(suggestion.getShardSize(),
-                                                                                                         gens.toArray(new CandidateGenerator[gens.size()])), suggestion.maxErrors(),
-                                                              suggestion.getShardSize(), wordScorer, suggestion.confidence(), suggestion.gramSize());
+                checkerResult = checker.getCorrections(stream,
+                        new MultiCandidateGeneratorWrapper(suggestion.getShardSize(), gens.toArray(new CandidateGenerator[gens.size()])),
+                        suggestion.maxErrors(), suggestion.getShardSize(), wordScorer, suggestion.confidence(), suggestion.gramSize());
                 }
 
             PhraseSuggestion.Entry resultEntry = buildResultEntry(suggestion, spare, checkerResult.cutoffScore);
@@ -152,10 +155,15 @@ private PhraseSuggestion.Entry buildResultEntry(PhraseSuggestionContext suggesti
     ScriptService scriptService() {
         return scriptService;
     }
-    
+
     @Override
     public SuggestContextParser getContextParser() {
         return new PhraseSuggestParser(this);
     }
 
+    @Override
+    public SuggestionBuilder<?> getBuilderPrototype() {
+        return PhraseSuggestionBuilder.PROTOTYPE;
+    }
+
 }