Improves the organization of the suggestion builder tests

core/src/main/java/org/elasticsearch/action/suggest/SuggestRequest.java

@@ -54,7 +54,7 @@ public final class SuggestRequest extends BroadcastRequest<SuggestRequest> {
     @Nullable
     private String preference;
 
-    private SuggestBuilder suggestSource;
+    private SuggestBuilder suggest;
 
     public SuggestRequest() {
     }
@@ -74,18 +74,18 @@ public final class SuggestRequest extends BroadcastRequest<SuggestRequest> {
     }
 
     /**
-     * The Phrase to get correction suggestions for
+     * The suggestion query to get correction suggestions for
      */
     public SuggestBuilder suggest() {
-        return suggestSource;
+        return suggest;
     }
 
     /**
      * set a new source for the suggest query
      */
-    public SuggestRequest suggest(SuggestBuilder suggestSource) {
+    public SuggestRequest suggest(SuggestBuilder suggest) {
-        Objects.requireNonNull(suggestSource, "suggestSource must not be null");
+        Objects.requireNonNull(suggest, "suggest must not be null");
-        this.suggestSource = suggestSource;
+        this.suggest = suggest;
         return this;
     }
 
@@ -126,29 +126,29 @@ public final class SuggestRequest extends BroadcastRequest<SuggestRequest> {
         super.readFrom(in);
         routing = in.readOptionalString();
         preference = in.readOptionalString();
-        suggest(SuggestBuilder.PROTOTYPE.readFrom(in));
+        suggest = SuggestBuilder.PROTOTYPE.readFrom(in);
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
-        Objects.requireNonNull(suggestSource, "suggestSource must not be null");
+        Objects.requireNonNull(suggest, "suggest must not be null");
         super.writeTo(out);
         out.writeOptionalString(routing);
         out.writeOptionalString(preference);
-        suggestSource.writeTo(out);
+        suggest.writeTo(out);
     }
 
     @Override
     public String toString() {
-        Objects.requireNonNull(suggestSource, "suggestSource must not be null");
+        Objects.requireNonNull(suggest, "suggest must not be null");
         String sSource = "_na_";
         try {
             XContentBuilder builder = JsonXContent.contentBuilder();
-            builder = suggestSource.toXContent(builder, ToXContent.EMPTY_PARAMS);
+            builder = suggest.toXContent(builder, ToXContent.EMPTY_PARAMS);
             sSource = builder.string();
         } catch (Exception e) {
             // ignore
         }
-        return "[" + Arrays.toString(indices) + "]" + ", suggestSource[" + sSource + "]";
+        return "[" + Arrays.toString(indices) + "]" + ", suggest[" + sSource + "]";
     }
 }

core/src/main/java/org/elasticsearch/rest/action/suggest/RestSuggestAction.java

@@ -81,7 +81,7 @@ public class RestSuggestAction extends BaseRestHandler {
                 final QueryParseContext context = new QueryParseContext(queryRegistry);
                 context.reset(parser);
                 context.parseFieldMatcher(parseFieldMatcher);
-                suggestRequest.suggest(SuggestBuilder.fromXContent(context, suggesters, true));
+                suggestRequest.suggest(SuggestBuilder.fromXContent(context, suggesters));
             }
         } else {
             throw new IllegalArgumentException("no content or source provided to execute suggestion");

core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java

@@ -851,7 +851,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ
                     XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().copyCurrentStructure(parser);
                     innerHitsBuilder = xContentBuilder.bytes();
                 } else if (context.parseFieldMatcher().match(currentFieldName, SUGGEST_FIELD)) {
-                    suggestBuilder = SuggestBuilder.fromXContent(context, suggesters, false);
+                    suggestBuilder = SuggestBuilder.fromXContent(context, suggesters);
                 } else if (context.parseFieldMatcher().match(currentFieldName, SORT_FIELD)) {
                     sorts = new ArrayList<>();
                     XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().copyCurrentStructure(parser);

core/src/main/java/org/elasticsearch/search/suggest/SuggestBuilder.java

@@ -112,18 +112,17 @@ public class SuggestBuilder extends ToXContentToBytes implements Writeable<Sugge
         return builder;
     }
 
-    public static SuggestBuilder fromXContent(QueryParseContext parseContext, Suggesters suggesters, final boolean fromStart)
+    public static SuggestBuilder fromXContent(QueryParseContext parseContext, Suggesters suggesters) throws IOException {
-        throws IOException {
-
         XContentParser parser = parseContext.parser();
         ParseFieldMatcher parseFieldMatcher = parseContext.parseFieldMatcher();
         SuggestBuilder suggestBuilder = new SuggestBuilder();
         String fieldName = null;
 
         XContentParser.Token token;
-        if (fromStart && parser.nextToken() != XContentParser.Token.START_OBJECT) {
+        if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
-            throw new IllegalArgumentException("Suggestion must start as an object");
+            parser.nextToken();
         }
+        assert parser.currentToken() == XContentParser.Token.START_OBJECT;
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.FIELD_NAME) {
                 fieldName = parser.currentName();

core/src/test/java/org/elasticsearch/search/suggest/AbstractSuggestionBuilderTestCase.java

@@ -46,6 +46,7 @@ import org.elasticsearch.index.mapper.core.StringFieldMapper.StringFieldType;
 import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.indices.IndicesModule;
+import org.elasticsearch.indices.query.IndicesQueriesRegistry;
 import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptContext;
@@ -54,6 +55,7 @@ import org.elasticsearch.script.ScriptEngineRegistry;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.ScriptServiceTests.TestEngineService;
 import org.elasticsearch.script.ScriptSettings;
+import org.elasticsearch.search.SearchModule;
 import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext;
 import org.elasticsearch.search.suggest.completion.CompletionSuggestionBuilder;
 import org.elasticsearch.search.suggest.phrase.PhraseSuggestionBuilder;
@@ -78,7 +80,9 @@ public abstract class AbstractSuggestionBuilderTestCase<SB extends SuggestionBui
 
     private static final int NUMBER_OF_TESTBUILDERS = 20;
     protected static NamedWriteableRegistry namedWriteableRegistry;
-    private static Suggesters suggesters;
+    protected static IndicesQueriesRegistry queriesRegistry;
+    protected static ParseFieldMatcher parseFieldMatcher;
+    protected static Suggesters suggesters;
     private static ScriptService scriptService;
     private static SuggestParseElement parseElement;
 
@@ -111,11 +115,15 @@ public abstract class AbstractSuggestionBuilderTestCase<SB extends SuggestionBui
         namedWriteableRegistry.registerPrototype(SuggestionBuilder.class, TermSuggestionBuilder.PROTOTYPE);
         namedWriteableRegistry.registerPrototype(SuggestionBuilder.class, PhraseSuggestionBuilder.PROTOTYPE);
         namedWriteableRegistry.registerPrototype(SuggestionBuilder.class, CompletionSuggestionBuilder.PROTOTYPE);
+        queriesRegistry = new SearchModule(Settings.EMPTY, namedWriteableRegistry).buildQueryParserRegistry();
+        parseFieldMatcher = ParseFieldMatcher.STRICT;
     }
 
     @AfterClass
     public static void afterClass() throws Exception {
         namedWriteableRegistry = null;
+        suggesters = null;
+        queriesRegistry = null;
     }
 
     /**
@@ -360,4 +368,11 @@ public abstract class AbstractSuggestionBuilderTestCase<SB extends SuggestionBui
             }
         }
     }
+
+    protected static QueryParseContext newParseContext(final String xcontent) throws IOException {
+        final QueryParseContext parseContext = new QueryParseContext(queriesRegistry);
+        parseContext.reset(XContentFactory.xContent(xcontent).createParser(xcontent));
+        parseContext.parseFieldMatcher(parseFieldMatcher);
+        return parseContext;
+    }
 }

core/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java

@@ -23,7 +23,6 @@ import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 
 import org.apache.lucene.analysis.TokenStreamToAutomaton;
 import org.apache.lucene.search.suggest.document.ContextSuggestField;
-import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
 import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeResponse;
 import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
@@ -80,8 +79,6 @@ import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 
-// nocommit
-@LuceneTestCase.AwaitsFix(bugUrl = "waiting on completion suggestion builder refactoring")
 @SuppressCodecs("*") // requires custom completion format
 public class CompletionSuggestSearchIT extends ESIntegTestCase {
     private final String INDEX = RandomStrings.randomAsciiOfLength(getRandom(), 10).toLowerCase(Locale.ROOT);

core/src/test/java/org/elasticsearch/search/suggest/ContextCompletionSuggestSearchIT.java

@@ -21,7 +21,6 @@ package org.elasticsearch.search.suggest;
 import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 
 import org.apache.lucene.spatial.util.GeoHashUtils;
-import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
 import org.elasticsearch.action.index.IndexRequestBuilder;
 import org.elasticsearch.action.suggest.SuggestResponse;
@@ -53,8 +52,6 @@ import java.util.Map;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 
-// nocommit
-@LuceneTestCase.AwaitsFix(bugUrl = "waiting on completion suggestion builder refactoring")
 @SuppressCodecs("*") // requires custom completion format
 public class ContextCompletionSuggestSearchIT extends ESIntegTestCase {
 

core/src/test/java/org/elasticsearch/search/suggest/SuggestBuilderTests.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.search.suggest;
 
-import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -31,8 +30,7 @@ import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.query.QueryParseContext;
-import org.elasticsearch.indices.query.IndicesQueriesRegistry;
+import org.elasticsearch.search.suggest.completion.CompletionSuggesterBuilderTests;
-import org.elasticsearch.search.SearchModule;
 import org.elasticsearch.search.suggest.completion.CompletionSuggestionBuilder;
 import org.elasticsearch.search.suggest.completion.WritableTestCase;
 import org.elasticsearch.search.suggest.phrase.Laplace;
@@ -48,18 +46,11 @@ import org.junit.BeforeClass;
 
 import java.io.IOException;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.Locale;
 import java.util.Map.Entry;
 
-import static org.hamcrest.Matchers.containsString;
-
 public class SuggestBuilderTests extends WritableTestCase<SuggestBuilder> {
 
     private static NamedWriteableRegistry namedWriteableRegistry;
-    private static IndicesQueriesRegistry queriesRegistry;
-    private static ParseFieldMatcher parseFieldMatcher;
-    private static Suggesters suggesters;
 
     /**
      * Setup for the whole base test class.
@@ -74,17 +65,11 @@ public class SuggestBuilderTests extends WritableTestCase<SuggestBuilder> {
         nwRegistry.registerPrototype(SmoothingModel.class, LinearInterpolation.PROTOTYPE);
         nwRegistry.registerPrototype(SmoothingModel.class, StupidBackoff.PROTOTYPE);
         namedWriteableRegistry = nwRegistry;
-        queriesRegistry = new SearchModule(Settings.EMPTY, namedWriteableRegistry).buildQueryParserRegistry();
-        suggesters = new Suggesters(new HashMap<>());
-        parseFieldMatcher = ParseFieldMatcher.STRICT;
     }
 
     @AfterClass
     public static void afterClass() {
         namedWriteableRegistry = null;
-        queriesRegistry = null;
-        suggesters = null;
-        parseFieldMatcher = null;
     }
 
     @Override
@@ -92,66 +77,6 @@ public class SuggestBuilderTests extends WritableTestCase<SuggestBuilder> {
         return namedWriteableRegistry;
     }
 
-    /**
-     * Test that valid JSON suggestion request passes.
-     */
-    public void testValidJsonRequestPayload() throws Exception {
-        final String field = RandomStrings.randomAsciiOfLength(getRandom(), 10).toLowerCase(Locale.ROOT);
-        String payload = "{\n" +
-                         "  \"valid-suggestion\" : {\n" +
-                         "    \"text\" : \"the amsterdma meetpu\",\n" +
-                         "    \"term\" : {\n" +
-                         "      \"field\" : \"" + field + "\"\n" +
-                         "    }\n" +
-                         "  }\n" +
-                         "}";
-        try {
-            final SuggestBuilder suggestBuilder = SuggestBuilder.fromXContent(newParseContext(payload), suggesters, true);
-            assertNotNull(suggestBuilder);
-        } catch (Exception e) {
-            fail("Parsing valid json should not have thrown exception: " + e.getMessage());
-        }
-    }
-
-    /**
-     * Test that a malformed JSON suggestion request fails.
-     */
-    public void testMalformedJsonRequestPayload() throws Exception {
-        final String field = RandomStrings.randomAsciiOfLength(getRandom(), 10).toLowerCase(Locale.ROOT);
-        // {"bad-payload":{"prefix":"sug","completion":{"field":"ytnahgylcc","payload":[{"payload":"field"}]}}}
-        String payload = "{\n" +
-                         "  \"bad-payload\" : {\n" +
-                         "    \"text\" : \"the amsterdma meetpu\",\n" +
-                         "    \"term\" : {\n" +
-                         "      \"field\" : { \"" + field + "\" : \"bad-object\" }\n" +
-                         "    }\n" +
-                         "  }\n" +
-                         "}";
-        try {
-            final SuggestBuilder suggestBuilder = SuggestBuilder.fromXContent(newParseContext(payload), suggesters, true);
-            fail("Should not have been able to create SuggestBuilder from malformed JSON: " + suggestBuilder);
-        } catch (Exception e) {
-            assertThat(e.getMessage(), containsString("parsing failed"));
-        }
-
-        // nocommit TODO: awaits completion suggester
-        /*payload = "{\n" +
-                  "  \"bad-payload\" : { \n" +
-                  "    \"prefix\" : \"sug\",\n" +
-                  "    \"completion\" : { \n" +
-                  "      \"field\" : \"" + field + "\",\n " +
-                  "      \"payload\" : [ {\"payload\":\"field\"} ]\n" +
-                  "    }\n" +
-                  "  }\n" +
-                  "}\n";
-        try {
-            final SuggestBuilder suggestBuilder = SuggestBuilder.fromXContent(newParseContext(payload), suggesters);
-            fail("Should not have been able to create SuggestBuilder from malformed JSON: " + suggestBuilder);
-        } catch (IllegalArgumentException e) {
-            assertThat(e.getMessage(), containsString("encountered invalid token"));
-        }*/
-    }
-
     /**
      *  creates random suggestion builder, renders it to xContent and back to new instance that should be equal to original
      */
@@ -169,7 +94,7 @@ public class SuggestBuilderTests extends WritableTestCase<SuggestBuilder> {
             XContentParser parser = XContentHelper.createParser(xContentBuilder.bytes());
             context.reset(parser);
 
-            SuggestBuilder secondSuggestBuilder = SuggestBuilder.fromXContent(context, suggesters, true);
+            SuggestBuilder secondSuggestBuilder = SuggestBuilder.fromXContent(context, suggesters);
             assertNotSame(suggestBuilder, secondSuggestBuilder);
             assertEquals(suggestBuilder, secondSuggestBuilder);
             assertEquals(suggestBuilder.hashCode(), secondSuggestBuilder.hashCode());
@@ -233,16 +158,9 @@ public class SuggestBuilderTests extends WritableTestCase<SuggestBuilder> {
         switch (randomIntBetween(0, 2)) {
             case 0: return TermSuggestionBuilderTests.randomTermSuggestionBuilder();
             case 1: return PhraseSuggestionBuilderTests.randomPhraseSuggestionBuilder();
-            //norelease TODO: uncomment case 2: return CompletionSuggesterBuilderTests.randomCompletionSuggestionBuilder();
+            case 2: return CompletionSuggesterBuilderTests.randomCompletionSuggestionBuilder();
             default: return TermSuggestionBuilderTests.randomTermSuggestionBuilder();
         }
     }
 
-    private static QueryParseContext newParseContext(final String xcontent) throws IOException {
-        final QueryParseContext parseContext = new QueryParseContext(queriesRegistry);
-        parseContext.reset(XContentFactory.xContent(xcontent).createParser(xcontent));
-        parseContext.parseFieldMatcher(parseFieldMatcher);
-        return parseContext;
-    }
-
 }

core/src/test/java/org/elasticsearch/search/suggest/completion/CompletionSuggesterBuilderTests.java

@@ -19,11 +19,13 @@
 
 package org.elasticsearch.search.suggest.completion;
 
+import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.core.CompletionFieldMapper;
 import org.elasticsearch.search.suggest.AbstractSuggestionBuilderTestCase;
+import org.elasticsearch.search.suggest.SuggestBuilder;
 import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext;
 import org.elasticsearch.search.suggest.completion.context.CategoryContextMapping;
 import org.elasticsearch.search.suggest.completion.context.CategoryQueryContext;
@@ -38,10 +40,12 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.core.IsInstanceOf.instanceOf;
+import static org.hamcrest.Matchers.containsString;
 
 public class CompletionSuggesterBuilderTests extends AbstractSuggestionBuilderTestCase<CompletionSuggestionBuilder> {
 
@@ -168,4 +172,26 @@ public class CompletionSuggesterBuilderTests extends AbstractSuggestionBuilderTe
                 throw new IllegalStateException("should not through");
         }
     }
+
+    /**
+     * Test that a malformed JSON suggestion request fails.
+     */
+    public void testMalformedJsonRequestPayload() throws Exception {
+        final String field = RandomStrings.randomAsciiOfLength(getRandom(), 10).toLowerCase(Locale.ROOT);
+        final String payload = "{\n" +
+                               "  \"bad-payload\" : { \n" +
+                               "    \"prefix\" : \"sug\",\n" +
+                               "    \"completion\" : { \n" +
+                               "      \"field\" : \"" + field + "\",\n " +
+                               "      \"payload\" : [ {\"payload\":\"field\"} ]\n" +
+                               "    }\n" +
+                               "  }\n" +
+                               "}\n";
+        try {
+            final SuggestBuilder suggestBuilder = SuggestBuilder.fromXContent(newParseContext(payload), suggesters);
+            fail("Should not have been able to create SuggestBuilder from malformed JSON: " + suggestBuilder);
+        } catch (IllegalArgumentException e) {
+            assertThat(e.getMessage(), containsString("parsing failed"));
+        }
+    }
 }

core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java

@@ -19,14 +19,17 @@
 
 package org.elasticsearch.search.suggest.term;
 
+import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 import org.elasticsearch.search.suggest.AbstractSuggestionBuilderTestCase;
 import org.elasticsearch.search.suggest.DirectSpellcheckerSettings;
 import org.elasticsearch.search.suggest.SortBy;
+import org.elasticsearch.search.suggest.SuggestBuilder;
 import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext;
 import org.elasticsearch.search.suggest.term.TermSuggestionBuilder.StringDistanceImpl;
 import org.elasticsearch.search.suggest.term.TermSuggestionBuilder.SuggestMode;
 
 import java.io.IOException;
+import java.util.Locale;
 
 import static org.elasticsearch.search.suggest.DirectSpellcheckerSettings.DEFAULT_ACCURACY;
 import static org.elasticsearch.search.suggest.DirectSpellcheckerSettings.DEFAULT_MAX_EDITS;
@@ -35,6 +38,7 @@ import static org.elasticsearch.search.suggest.DirectSpellcheckerSettings.DEFAUL
 import static org.elasticsearch.search.suggest.DirectSpellcheckerSettings.DEFAULT_MIN_DOC_FREQ;
 import static org.elasticsearch.search.suggest.DirectSpellcheckerSettings.DEFAULT_MIN_WORD_LENGTH;
 import static org.elasticsearch.search.suggest.DirectSpellcheckerSettings.DEFAULT_PREFIX_LENGTH;
+import static org.hamcrest.Matchers.containsString;
 
 /**
  * Test the {@link TermSuggestionBuilder} class.
@@ -280,6 +284,24 @@ public class TermSuggestionBuilderTests extends AbstractSuggestionBuilderTestCas
         assertEquals(SuggestMode.MISSING, builder.suggestMode());
     }
 
+    public void testMalformedJson() {
+        final String field = RandomStrings.randomAsciiOfLength(getRandom(), 10).toLowerCase(Locale.ROOT);
+        String suggest = "{\n" +
+                         "  \"bad-payload\" : {\n" +
+                         "    \"text\" : \"the amsterdma meetpu\",\n" +
+                         "    \"term\" : {\n" +
+                         "      \"field\" : { \"" + field + "\" : \"bad-object\" }\n" +
+                         "    }\n" +
+                         "  }\n" +
+                         "}";
+        try {
+            final SuggestBuilder suggestBuilder = SuggestBuilder.fromXContent(newParseContext(suggest), suggesters);
+            fail("Should not have been able to create SuggestBuilder from malformed JSON: " + suggestBuilder);
+        } catch (Exception e) {
+            assertThat(e.getMessage(), containsString("parsing failed"));
+        }
+    }
+
     @Override
     protected void assertSuggestionContext(SuggestionContext oldSuggestion, SuggestionContext newSuggestion) {
         @SuppressWarnings("unchecked")