From 73b819bf9ba13649a1ebaa6b24f4361864bea98b Mon Sep 17 00:00:00 2001 From: Ali Beyad Date: Tue, 9 Feb 2016 00:25:47 -0500 Subject: [PATCH] Building the term suggesters from the builder object --- .../suggest/TransportSuggestAction.java | 4 +- .../suggest/DirectSpellcheckerSettings.java | 30 ++++++++--- .../elasticsearch/search/suggest/Suggest.java | 34 ------------- .../search/suggest/SuggestBuilder.java | 9 ++++ .../search/suggest/SuggestParseElement.java | 2 +- .../search/suggest/SuggestUtils.java | 31 ++++++++++-- .../search/suggest/SuggestionBuilder.java | 17 ++++++- .../suggest/SuggestionSearchContext.java | 15 ++++++ .../CompletionSuggestionBuilder.java | 2 +- .../DirectCandidateGeneratorBuilder.java | 4 +- .../phrase/PhraseSuggestionBuilder.java | 10 +--- .../search/suggest/term/TermSuggestion.java | 7 +-- .../suggest/term/TermSuggestionBuilder.java | 50 ++++++++++++++++++- .../suggest/term/TermSuggestionContext.java | 7 ++- .../AbstractSuggestionBuilderTestCase.java | 11 +++- .../suggest/CustomSuggesterSearchIT.java | 10 ++-- .../term/TermSuggestionBuilderTests.java | 46 +++++++++++------ 17 files changed, 202 insertions(+), 87 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/suggest/TransportSuggestAction.java b/core/src/main/java/org/elasticsearch/action/suggest/TransportSuggestAction.java index 36fb079d257..616dbf94937 100644 --- a/core/src/main/java/org/elasticsearch/action/suggest/TransportSuggestAction.java +++ b/core/src/main/java/org/elasticsearch/action/suggest/TransportSuggestAction.java @@ -39,7 +39,6 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.engine.Engine; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.suggest.stats.ShardSuggestMetric; import org.elasticsearch.indices.IndicesService; @@ -132,7 +131,6 @@ public class TransportSuggestAction extends TransportBroadcastAction LUCENE_FREQUENCY = new SuggestWordFrequencyComparator(); @@ -172,12 +173,12 @@ public final class SuggestUtils { } } - public static Suggest.Suggestion.Sort resolveSort(String sortVal) { + public static TermSuggestionBuilder.SortBy resolveSort(String sortVal) { sortVal = sortVal.toLowerCase(Locale.US); if ("score".equals(sortVal)) { - return Suggest.Suggestion.Sort.SCORE; + return TermSuggestionBuilder.SortBy.SCORE; } else if ("frequency".equals(sortVal)) { - return Suggest.Suggestion.Sort.FREQUENCY; + return TermSuggestionBuilder.SortBy.FREQUENCY; } else { throw new IllegalArgumentException("Illegal suggest sort " + sortVal); } @@ -201,6 +202,28 @@ public final class SuggestUtils { } } + public static SuggestMode resolveSuggestMode(TermSuggestionBuilder.SuggestMode suggestMode) { + Objects.requireNonNull(suggestMode, "suggestMode must not be null"); + switch (suggestMode) { + case MISSING: return SuggestMode.SUGGEST_WHEN_NOT_IN_INDEX; + case POPULAR: return SuggestMode.SUGGEST_MORE_POPULAR; + case ALWAYS: return SuggestMode.SUGGEST_ALWAYS; + default: throw new IllegalArgumentException("Unknown suggestMode [" + suggestMode + "]"); + } + } + + public static StringDistance resolveStringDistance(TermSuggestionBuilder.StringDistanceImpl stringDistance) { + Objects.requireNonNull(stringDistance, "stringDistance must not be null"); + switch (stringDistance) { + case INTERNAL: return DirectSpellChecker.INTERNAL_LEVENSHTEIN; + case DAMERAU_LEVENSHTEIN: return new LuceneLevenshteinDistance(); + case LEVENSTEIN: return new LevensteinDistance(); + case JAROWINKLER: return new JaroWinklerDistance(); + case NGRAM: return new NGramDistance(); + default: throw new IllegalArgumentException("Illegal distance option " + stringDistance); + } + } + public static class Fields { public static final ParseField STRING_DISTANCE = new ParseField("string_distance"); public static final ParseField SUGGEST_MODE = new ParseField("suggest_mode"); @@ -243,7 +266,7 @@ public final class SuggestUtils { } else if (parseFieldMatcher.match(fieldName, Fields.PREFIX_LENGTH)) { suggestion.prefixLength(parser.intValue()); } else if (parseFieldMatcher.match(fieldName, Fields.MIN_WORD_LENGTH)) { - suggestion.minQueryLength(parser.intValue()); + suggestion.minWordLength(parser.intValue()); } else if (parseFieldMatcher.match(fieldName, Fields.MIN_DOC_FREQ)) { suggestion.minDocFreq(parser.floatValue()); } else { diff --git a/core/src/main/java/org/elasticsearch/search/suggest/SuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/SuggestionBuilder.java index e1ecca1ccce..fd973e48f65 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/SuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/SuggestionBuilder.java @@ -19,7 +19,9 @@ package org.elasticsearch.search.suggest; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.action.support.ToXContentToBytes; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.io.stream.NamedWriteable; @@ -194,7 +196,20 @@ public abstract class SuggestionBuilder> extends protected abstract SuggestionBuilder innerFromXContent(QueryParseContext parseContext, String name) throws IOException; - protected abstract SuggestionContext build(QueryShardContext context) throws IOException; + public SuggestionContext build(QueryShardContext context, @Nullable String globalText) throws IOException { + SuggestionContext suggestionContext = innerBuild(context); + // copy over common settings to each suggestion builder + SuggestUtils.suggestionToSuggestionContext(this, context.getMapperService(), suggestionContext); + SuggestUtils.verifySuggestion(context.getMapperService(), new BytesRef(globalText), suggestionContext); + suggestionContext.setShardContext(context); + // TODO make field mandatory in the builder, then remove this + if (suggestionContext.getField() == null) { + throw new IllegalArgumentException("The required field option is missing"); + } + return suggestionContext; + } + + protected abstract SuggestionContext innerBuild(QueryShardContext context) throws IOException; public String getSuggesterName() { //default impl returns the same as writeable name, but we keep the distinction between the two just to make sure diff --git a/core/src/main/java/org/elasticsearch/search/suggest/SuggestionSearchContext.java b/core/src/main/java/org/elasticsearch/search/suggest/SuggestionSearchContext.java index 02953356876..206b8811792 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/SuggestionSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/SuggestionSearchContext.java @@ -127,6 +127,21 @@ public class SuggestionSearchContext { public QueryShardContext getShardContext() { return this.shardContext; } + + @Override + public String toString() { + return "[" + + "text=" + text + + ",field=" + field + + ",prefix=" + prefix + + ",regex=" + regex + + ",size=" + size + + ",shardSize=" + shardSize + + ",suggester=" + suggester + + ",analyzer=" + analyzer + + ",shardContext=" + shardContext + + "]"; + } } } diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java index 0bd37be128d..1f4a5cda9c3 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java @@ -379,7 +379,7 @@ public class CompletionSuggestionBuilder extends SuggestionBuilder { } public static final int TYPE = 1; - private Sort sort; + private TermSuggestionBuilder.SortBy sort; public TermSuggestion() { } - public TermSuggestion(String name, int size, Sort sort) { + public TermSuggestion(String name, int size, TermSuggestionBuilder.SortBy sort) { super(name, size); this.sort = sort; } @@ -110,7 +111,7 @@ public class TermSuggestion extends Suggestion { @Override protected void innerReadFrom(StreamInput in) throws IOException { super.innerReadFrom(in); - sort = Sort.fromId(in.readByte()); + sort = TermSuggestionBuilder.SortBy.fromId(in.readByte()); } @Override diff --git a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java index 1f190b674a6..3988da23e56 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java @@ -20,6 +20,7 @@ package org.elasticsearch.search.suggest.term; import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -27,6 +28,8 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.search.suggest.DirectSpellcheckerSettings; +import org.elasticsearch.search.suggest.SuggestUtils; import org.elasticsearch.search.suggest.SuggestionBuilder; import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext; @@ -383,6 +386,12 @@ public class TermSuggestionBuilder extends SuggestionBuilder { /** Only suggest terms in the suggest text that aren't in the index. This is the default. */ @@ -472,12 +498,18 @@ public class TermSuggestionBuilder extends SuggestionBuilder { /** Sort should first be based on score, then document frequency and then the term itself. */ - SCORE, + SCORE((byte) 0x0), /** Sort should first be based on document frequency, then score and then the term itself. */ - FREQUENCY; + FREQUENCY((byte) 0x1); protected static SortBy PROTOTYPE = SCORE; + private byte id; + + SortBy(byte id) { + this.id = id; + } + @Override public void writeTo(final StreamOutput out) throws IOException { out.writeVInt(ordinal()); @@ -496,6 +528,20 @@ public class TermSuggestionBuilder extends SuggestionBuilder options = new HashMap<>(); + options.put("field", randomField); + options.put("suffix", randomSuffix); + return new CustomSuggester.CustomSuggestionsContext(new CustomSuggester(), options); } } diff --git a/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java b/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java index ac14efdb4d1..7085ed415d2 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java @@ -20,7 +20,11 @@ package org.elasticsearch.search.suggest.term; import org.elasticsearch.search.suggest.AbstractSuggestionBuilderTestCase; +import org.elasticsearch.search.suggest.DirectSpellcheckerSettings; import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext; +import org.elasticsearch.search.suggest.term.TermSuggestionBuilder.SortBy; +import org.elasticsearch.search.suggest.term.TermSuggestionBuilder.StringDistanceImpl; +import org.elasticsearch.search.suggest.term.TermSuggestionBuilder.SuggestMode; import java.io.IOException; @@ -31,20 +35,9 @@ import static org.hamcrest.Matchers.notNullValue; */ public class TermSuggestionBuilderTests extends AbstractSuggestionBuilderTestCase { - /** - * creates random suggestion builder, renders it to xContent and back to new instance that should be equal to original - */ @Override - public void testFromXContent() throws IOException { - // skip for now - } - - /** - * creates random suggestion builder, renders it to xContent and back to new instance that should be equal to original - */ - @Override - public void testBuild() throws IOException { - // skip for now + public void testFromXContent() { + // NORELEASE : remove this when TermSuggestionBuilder's fromXContent is in } @Override @@ -261,7 +254,32 @@ public class TermSuggestionBuilderTests extends AbstractSuggestionBuilderTestCas @Override protected void assertSuggestionContext(SuggestionContext oldSuggestion, SuggestionContext newSuggestion) { - // put assertions on TermSuggestionContext here + @SuppressWarnings("unchecked") + TermSuggestionContext oldContext = (TermSuggestionContext) oldSuggestion; + @SuppressWarnings("unchecked") + TermSuggestionContext newContext = (TermSuggestionContext) newSuggestion; + assertSpellcheckerSettings(oldContext.getDirectSpellCheckerSettings(), newContext.getDirectSpellCheckerSettings()); + + } + + private void assertSpellcheckerSettings(DirectSpellcheckerSettings oldSettings, DirectSpellcheckerSettings newSettings) { + final double delta = 0.0d; + // make sure the objects aren't the same + assertNotSame(oldSettings, newSettings); + // make sure the objects aren't null + assertNotNull(oldSettings); + assertNotNull(newSettings); + // and now, make sure they are equal.. + assertEquals(oldSettings.accuracy(), newSettings.accuracy(), delta); + assertEquals(oldSettings.maxEdits(), newSettings.maxEdits()); + assertEquals(oldSettings.maxInspections(), newSettings.maxInspections()); + assertEquals(oldSettings.maxTermFreq(), newSettings.maxTermFreq(), delta); + assertEquals(oldSettings.minDocFreq(), newSettings.minDocFreq(), delta); + assertEquals(oldSettings.minWordLength(), newSettings.minWordLength()); + assertEquals(oldSettings.prefixLength(), newSettings.prefixLength()); + assertEquals(oldSettings.sort(), newSettings.sort()); + assertEquals(oldSettings.stringDistance().getClass(), newSettings.stringDistance().getClass()); + assertEquals(oldSettings.suggestMode().getClass(), newSettings.suggestMode().getClass()); } }