Addressing review comments

core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java

@@ -33,11 +33,8 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryShardContext;
-import org.elasticsearch.search.SearchParseException;
 import org.elasticsearch.search.highlight.SearchContextHighlight.FieldOptions;
 import org.elasticsearch.search.highlight.SearchContextHighlight.FieldOptions.Builder;
-import org.elasticsearch.search.internal.SearchContext;
-
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -84,14 +81,14 @@ public class HighlightBuilder extends AbstractHighlighterBuilder<HighlightBuilde
     public static final String[] DEFAULT_POST_TAGS = new String[]{"</em>"};
 
     /** the default opening tags when <tt>tag_schema = "styled"</tt>  */
-    public static final String[] STYLED_PRE_TAG = {
+    public static final String[] DEFAULT_STYLED_PRE_TAG = {
             "<em class=\"hlt1\">", "<em class=\"hlt2\">", "<em class=\"hlt3\">",
             "<em class=\"hlt4\">", "<em class=\"hlt5\">", "<em class=\"hlt6\">",
             "<em class=\"hlt7\">", "<em class=\"hlt8\">", "<em class=\"hlt9\">",
             "<em class=\"hlt10\">"
     };
     /** the default closing tags when <tt>tag_schema = "styled"</tt>  */
-    public static final String[] STYLED_POST_TAGS = {"</em>"};
+    public static final String[] DEFAULT_STYLED_POST_TAGS = {"</em>"};
 
     /**
      * a {@link FieldOptions.Builder} with default settings
@@ -181,8 +178,8 @@ public class HighlightBuilder extends AbstractHighlighterBuilder<HighlightBuilde
             postTags(DEFAULT_POST_TAGS);
             break;
         case "styled":
-            preTags(STYLED_PRE_TAG);
+            preTags(DEFAULT_STYLED_PRE_TAG);
-            postTags(STYLED_POST_TAGS);
+            postTags(DEFAULT_STYLED_POST_TAGS);
             break;
         default:
             throw new IllegalArgumentException("Unknown tag schema ["+ schemaName +"]");
@@ -346,15 +343,7 @@ public class HighlightBuilder extends AbstractHighlighterBuilder<HighlightBuilde
         return highlightBuilder;
     }
 
-    public void parse(XContentParser parser, SearchContext context) throws Exception {
+    public SearchContextHighlight build(QueryShardContext context) throws IOException {
-        try {
-            context.highlight(build(context.indexShard().getQueryShardContext()));
-        } catch (IllegalArgumentException ex) {
-            throw new SearchParseException(context, "Error while trying to parse Highlighter element in request", parser.getTokenLocation());
-        }
-    }
-
-    SearchContextHighlight build(QueryShardContext context) throws IOException {
         // create template global options that are later merged with any partial field options
         final SearchContextHighlight.FieldOptions.Builder globalOptionsBuilder = new SearchContextHighlight.FieldOptions.Builder();
         globalOptionsBuilder.encoder(this.encoder);

core/src/main/java/org/elasticsearch/search/highlight/HighlighterParseElement.java

@@ -108,8 +108,8 @@ public class HighlighterParseElement implements SearchParseElement {
                 } else if ("tags_schema".equals(topLevelFieldName) || "tagsSchema".equals(topLevelFieldName)) {
                     String schema = parser.text();
                     if ("styled".equals(schema)) {
-                        globalOptionsBuilder.preTags(HighlightBuilder.STYLED_PRE_TAG);
+                        globalOptionsBuilder.preTags(HighlightBuilder.DEFAULT_STYLED_PRE_TAG);
-                        globalOptionsBuilder.postTags(HighlightBuilder.STYLED_POST_TAGS);
+                        globalOptionsBuilder.postTags(HighlightBuilder.DEFAULT_STYLED_POST_TAGS);
                     }
                 } else if ("highlight_filter".equals(topLevelFieldName) || "highlightFilter".equals(topLevelFieldName)) {
                     globalOptionsBuilder.highlightFilter(parser.booleanValue());

core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java

@@ -36,6 +36,13 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexSettings;
+import org.elasticsearch.index.mapper.ContentPath;
+import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.mapper.Mapper;
+import org.elasticsearch.index.mapper.MapperBuilders;
+import org.elasticsearch.index.mapper.core.StringFieldMapper;
+import org.elasticsearch.index.query.IdsQueryBuilder;
+import org.elasticsearch.index.query.IdsQueryParser;
 import org.elasticsearch.index.query.MatchAllQueryBuilder;
 import org.elasticsearch.index.query.MatchAllQueryParser;
 import org.elasticsearch.index.query.QueryBuilder;
@@ -43,6 +50,7 @@ import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryParser;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.index.query.TermQueryBuilder;
+import org.elasticsearch.index.query.TermQueryParser;
 import org.elasticsearch.indices.query.IndicesQueriesRegistry;
 import org.elasticsearch.search.highlight.HighlightBuilder;
 import org.elasticsearch.search.highlight.HighlightBuilder.Field;
@@ -79,6 +87,8 @@ public class HighlightBuilderTests extends ESTestCase {
         @SuppressWarnings("rawtypes")
         Set<QueryParser> injectedQueryParsers = new HashSet<>();
         injectedQueryParsers.add(new MatchAllQueryParser());
+        injectedQueryParsers.add(new IdsQueryParser());
+        injectedQueryParsers.add(new TermQueryParser());
         indicesQueriesRegistry = new IndicesQueriesRegistry(Settings.settingsBuilder().build(), injectedQueryParsers, namedWriteableRegistry);
     }
 
@@ -277,7 +287,14 @@ public class HighlightBuilderTests extends ESTestCase {
         Index index = new Index(randomAsciiOfLengthBetween(1, 10));
         IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(index, indexSettings);
         // shard context will only need indicesQueriesRegistry for building Query objects nested in highlighter
-        QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, null, indicesQueriesRegistry);
+        QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, null, indicesQueriesRegistry) {
+            @Override
+            public MappedFieldType fieldMapper(String name) {
+                StringFieldMapper.Builder builder = MapperBuilders.stringField(name);
+                return builder.build(new Mapper.BuilderContext(idxSettings.getSettings(), new ContentPath(1))).fieldType();
+            }
+        };
+        mockShardContext.setMapUnmappedFieldAsString(true);
 
         for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
             HighlightBuilder highlightBuilder = randomHighlighterBuilder();
@@ -338,9 +355,9 @@ public class HighlightBuilderTests extends ESTestCase {
 
         context.reset(parser);
         HighlightBuilder highlightBuilder = HighlightBuilder.fromXContent(context);
-        assertArrayEquals("setting tags_schema 'styled' should alter pre_tags", HighlightBuilder.STYLED_PRE_TAG,
+        assertArrayEquals("setting tags_schema 'styled' should alter pre_tags", HighlightBuilder.DEFAULT_STYLED_PRE_TAG,
                 highlightBuilder.preTags());
-        assertArrayEquals("setting tags_schema 'styled' should alter post_tags", HighlightBuilder.STYLED_POST_TAGS,
+        assertArrayEquals("setting tags_schema 'styled' should alter post_tags", HighlightBuilder.DEFAULT_STYLED_POST_TAGS,
                 highlightBuilder.postTags());
 
         highlightElement = "{\n" +
@@ -423,9 +440,20 @@ public class HighlightBuilderTests extends ESTestCase {
             highlightBuilder.fragmenter(randomAsciiOfLengthBetween(1, 10));
         }
         if (randomBoolean()) {
-            QueryBuilder highlightQuery = new MatchAllQueryBuilder();
+            QueryBuilder highlightQuery;
+            switch (randomInt(2)) {
+            case 0:
+                highlightQuery = new MatchAllQueryBuilder();
+                break;
+            case 1:
+                highlightQuery = new IdsQueryBuilder();
+                break;
+            default:
+            case 2:
+                highlightQuery = new TermQueryBuilder(randomAsciiOfLengthBetween(1, 10), randomAsciiOfLengthBetween(1, 10));
+                break;
+            }
             highlightQuery.boost((float) randomDoubleBetween(0, 10, false));
-            highlightQuery.queryName(randomAsciiOfLength(10));
             highlightBuilder.highlightQuery(highlightQuery);
         }
         if (randomBoolean()) {