From c7780e6e0a00bd90265a7313badf2e73e8b06467 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 28 Mar 2016 09:05:56 -0400 Subject: [PATCH] Use ObjectParser in highlighting --- .../resources/checkstyle_suppressions.xml | 5 - .../common/xcontent/ObjectParser.java | 220 +++++++++++++--- .../index/query/support/InnerHitBuilder.java | 9 +- .../metrics/tophits/TopHitsParser.java | 2 +- .../search/builder/SearchSourceBuilder.java | 2 +- .../highlight/AbstractHighlighterBuilder.java | 139 ++++------- .../search/highlight/HighlightBuilder.java | 119 +++------ .../common/xcontent/ObjectParserTests.java | 196 ++++++++++++--- .../highlight/HighlightBuilderTests.java | 236 ++++++++++-------- 9 files changed, 568 insertions(+), 360 deletions(-) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index b2331039411..5055538bf71 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -373,7 +373,6 @@ - @@ -828,9 +827,7 @@ - - @@ -1051,7 +1048,6 @@ - @@ -1347,7 +1343,6 @@ - diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java index 395dcad8221..311b93744ed 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; import java.io.IOException; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; @@ -31,17 +32,37 @@ import java.util.List; import java.util.Map; import java.util.function.BiConsumer; import java.util.function.BiFunction; +import java.util.function.Consumer; import java.util.function.Supplier; +import static org.elasticsearch.common.xcontent.XContentParser.Token.START_ARRAY; +import static org.elasticsearch.common.xcontent.XContentParser.Token.START_OBJECT; +import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_BOOLEAN; +import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_EMBEDDED_OBJECT; +import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_NULL; +import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_NUMBER; +import static org.elasticsearch.common.xcontent.XContentParser.Token.VALUE_STRING; + /** - * A declarative Object parser to parse any kind of XContent structures into existing object with setters. - * The Parser is designed to be declarative and stateless. A single parser is defined for one object level, nested - * elements can be added via {@link #declareObject(BiConsumer, BiFunction, ParseField)} which is commonly done by - * declaring yet another instance of {@link ObjectParser}. Instances of {@link ObjectParser} are thread-safe and can be - * re-used across parsing operations. It's recommended to use the high level declare methods like {@link #declareString(BiConsumer, ParseField)} - * instead of {@link #declareField} which can be used to implement exceptional parsing operations not covered by the high level methods. + * A declarative Object parser to parse any kind of XContent structures into existing object with setters. The Parser is designed to be + * declarative and stateless. A single parser is defined for one object level, nested elements can be added via + * {@link #declareObject(BiConsumer, BiFunction, ParseField)} which is commonly done by declaring yet another instance of + * {@link ObjectParser}. Instances of {@link ObjectParser} are thread-safe and can be re-used across parsing operations. It's recommended to + * use the high level declare methods like {@link #declareString(BiConsumer, ParseField)} instead of {@link #declareField} which can be used + * to implement exceptional parsing operations not covered by the high level methods. */ public final class ObjectParser implements BiFunction { + /** + * Adapts an array (or varags) setter into a list setter. + */ + public static BiConsumer> fromList(Class c, + BiConsumer consumer) { + return (Value v, List l) -> { + @SuppressWarnings("unchecked") + ElementValue[] array = (ElementValue[]) Array.newInstance(c, l.size()); + consumer.accept(v, l.toArray(array)); + }; + } private final String name; private final Supplier valueSupplier; @@ -125,12 +146,14 @@ public final class ObjectParser implements BiFunction fieldParser, String currentFieldName, Value value, Context context) throws IOException { + private void parseArray(XContentParser parser, FieldParser fieldParser, String currentFieldName, Value value, Context context) + throws IOException { assert parser.currentToken() == XContentParser.Token.START_ARRAY : "Token was: " + parser.currentToken(); parseValue(parser, fieldParser, currentFieldName, value, context); } - private void parseValue(XContentParser parser, FieldParser fieldParser, String currentFieldName, Value value, Context context) throws IOException { + private void parseValue(XContentParser parser, FieldParser fieldParser, String currentFieldName, Value value, Context context) + throws IOException { try { fieldParser.parser.parse(parser, value, context); } catch (Exception ex) { @@ -138,7 +161,8 @@ public final class ObjectParser implements BiFunction fieldParser, String currentFieldName, Value value, Context context) throws IOException { + private void parseSub(XContentParser parser, FieldParser fieldParser, String currentFieldName, Value value, Context context) + throws IOException { final XContentParser.Token token = parser.currentToken(); switch (token) { case START_OBJECT: @@ -237,12 +261,14 @@ public final class ObjectParser implements BiFunction consumer.accept(v, objectParser.apply(p, c)), field, ValueType.OBJECT); } - public void declareObjectArray(BiConsumer> consumer, BiFunction objectParser, ParseField field) { + public void declareObjectArray(BiConsumer> consumer, BiFunction objectParser, + ParseField field) { declareField((p, v, c) -> consumer.accept(v, parseArray(p, () -> objectParser.apply(p, c))), field, ValueType.OBJECT_ARRAY); } - public void declareObjectOrDefault(BiConsumer consumer, BiFunction objectParser, Supplier defaultValue, ParseField field) { + public void declareObjectOrDefault(BiConsumer consumer, BiFunction objectParser, + Supplier defaultValue, ParseField field) { declareField((p, v, c) -> { if (p.currentToken() == XContentParser.Token.VALUE_BOOLEAN) { if (p.booleanValue()) { @@ -251,7 +277,7 @@ public final class ObjectParser implements BiFunction implements BiFunction consumer, ParseField field) { - declareField((p, v, c) -> consumer.accept(v, p.currentToken() == XContentParser.Token.VALUE_NULL ? null : p.text()), field, ValueType.STRING_OR_NULL); + declareField((p, v, c) -> consumer.accept(v, p.currentToken() == XContentParser.Token.VALUE_NULL ? null : p.text()), field, + ValueType.STRING_OR_NULL); } public void declareBoolean(BiConsumer consumer, ParseField field) { declareField((p, v, c) -> consumer.accept(v, p.booleanValue()), field, ValueType.BOOLEAN); } + /** + * Declares named objects in the style of highlighting's field element. These are usually named inside and object like this: + * 

+     * {
+     *   "highlight": {
+     *     "fields": {        <------ this one
+     *       "title": {},
+     *       "body": {},
+     *       "category": {}
+     *     }
+     *   }
+     * }
+     *
+ * but, when order is important, some may be written this way: + * 

+     * {
+     *   "highlight": {
+     *     "fields": [        <------ this one
+     *       {"title": {}},
+     *       {"body": {}},
+     *       {"category": {}}
+     *     ]
+     *   }
+     * }
+     *
+ * This is because json doesn't enforce ordering. Elasticsearch reads it in the order sent but tools that generate json are free to put + * object members in an unordered Map, jumbling them. Thus, if you care about order you can send the object in the second way. + * + * See NamedObjectHolder in ObjectParserTests for examples of how to invoke this. + * + * @param consumer sets the values once they have been parsed + * @param namedObjectParser parses each named object + * @param orderedModeCallback called when the named object is parsed using the "ordered" mode (the array of objects) + * @param field the field to parse + */ + public void declareNamedObjects(BiConsumer> consumer, NamedObjectParser namedObjectParser, + Consumer orderedModeCallback, ParseField field) { + // This creates and parses the named object + BiFunction objectParser = (XContentParser p, Context c) -> { + if (p.currentToken() != XContentParser.Token.FIELD_NAME) { + throw new ParsingException(p.getTokenLocation(), "[" + field + "] can be a single object with any number of " + + "fields or an array where each entry is an object with a single field"); + } + // This messy exception nesting has the nice side effect of telling the use which field failed to parse + try { + String name = p.currentName(); + try { + return namedObjectParser.parse(p, c, name); + } catch (Exception e) { + throw new ParsingException(p.getTokenLocation(), "[" + field + "] failed to parse field [" + name + "]", e); + } + } catch (IOException e) { + throw new ParsingException(p.getTokenLocation(), "[" + field + "] error while parsing", e); + } + }; + declareField((XContentParser p, Value v, Context c) -> { + List fields = new ArrayList<>(); + XContentParser.Token token; + if (p.currentToken() == XContentParser.Token.START_OBJECT) { + // Fields are just named entries in a single object + while ((token = p.nextToken()) != XContentParser.Token.END_OBJECT) { + fields.add(objectParser.apply(p, c)); + } + } else if (p.currentToken() == XContentParser.Token.START_ARRAY) { + // Fields are objects in an array. Each object contains a named field. + orderedModeCallback.accept(v); + while ((token = p.nextToken()) != XContentParser.Token.END_ARRAY) { + if (token != XContentParser.Token.START_OBJECT) { + throw new ParsingException(p.getTokenLocation(), "[" + field + "] can be a single object with any number of " + + "fields or an array where each entry is an object with a single field"); + } + p.nextToken(); // Move to the first field in the object + fields.add(objectParser.apply(p, c)); + p.nextToken(); // Move past the object, should be back to into the array + if (p.currentToken() != XContentParser.Token.END_OBJECT) { + throw new ParsingException(p.getTokenLocation(), "[" + field + "] can be a single object with any number of " + + "fields or an array where each entry is an object with a single field"); + } + } + } + consumer.accept(v, fields); + }, field, ValueType.OBJECT_ARRAY); + } + + /** + * Declares named objects in the style of aggregations. These are named inside and object like this: + * 

+     * {
+     *   "aggregations": {
+     *     "name_1": { "aggregation_type": {} },
+     *     "name_2": { "aggregation_type": {} },
+     *     "name_3": { "aggregation_type": {} }
+     *     }
+     *   }
+     * }
+     *
+ * Unlike the other version of this method, "ordered" mode (arrays of objects) is not supported. + * + * See NamedObjectHolder in ObjectParserTests for examples of how to invoke this. + * + * @param consumer sets the values once they have been parsed + * @param namedObjectParser parses each named object + * @param field the field to parse + */ + public void declareNamedObjects(BiConsumer> consumer, NamedObjectParser namedObjectParser, + ParseField field) { + Consumer orderedModeCallback = (Value v) -> { + throw new IllegalArgumentException("[" + field + "] doesn't support arrays. Use a single object with multiple fields."); + }; + declareNamedObjects(consumer, namedObjectParser, orderedModeCallback, field); + } + + /** + * Functional interface for instantiating and parsing named objects. See ObjectParserTests#NamedObject for the canonical way to + * implement this for objects that themselves have a parser. + */ + @FunctionalInterface + public interface NamedObjectParser { + T parse(XContentParser p, Context c, String name) throws IOException; + } + public static class FieldParser { private final Parser parser; private final EnumSet supportedTokens; @@ -305,7 +453,8 @@ public final class ObjectParser implements BiFunction implements BiFunction 0) { + deprecated = ", deprecated_names=" + Arrays.toString(deprecatedNames); + } return "FieldParser{" + "preferred_name=" + parseField.getPreferredName() + ", supportedTokens=" + supportedTokens + - (deprecatedNames == null || deprecatedNames.length == 0 ? "" : ", deprecated_names=" + Arrays.toString(deprecatedNames )) + + deprecated + (allReplacedWith == null ? "" : ", replaced_with=" + allReplacedWith) + ", type=" + type.name() + '}'; @@ -325,27 +478,28 @@ public final class ObjectParser implements BiFunction tokens; - ValueType(EnumSet tokens) { - this.tokens = tokens; + ValueType(XContentParser.Token first, XContentParser.Token... rest) { + this.tokens = EnumSet.of(first, rest); } public EnumSet supportedTokens() { diff --git a/core/src/main/java/org/elasticsearch/index/query/support/InnerHitBuilder.java b/core/src/main/java/org/elasticsearch/index/query/support/InnerHitBuilder.java index 2579a9171c3..fc60eb72c9f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/support/InnerHitBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/support/InnerHitBuilder.java @@ -98,13 +98,8 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl throw new ParsingException(p.getTokenLocation(), "Could not parse inner _source definition", e); } }, SearchSourceBuilder._SOURCE_FIELD, ObjectParser.ValueType.OBJECT_OR_BOOLEAN); - PARSER.declareObject(InnerHitBuilder::setHighlightBuilder, (p, c) -> { - try { - return HighlightBuilder.PROTOTYPE.fromXContent(c); - } catch (IOException e) { - throw new ParsingException(p.getTokenLocation(), "Could not parse inner highlight definition", e); - } - }, SearchSourceBuilder.HIGHLIGHT_FIELD); + PARSER.declareObject(InnerHitBuilder::setHighlightBuilder, (p, c) -> HighlightBuilder.fromXContent(c), + SearchSourceBuilder.HIGHLIGHT_FIELD); PARSER.declareObject(InnerHitBuilder::setQuery, (p, c) ->{ try { return c.parseInnerQueryBuilder(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsParser.java index 9ec6b46dafe..a12c9ece95f 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsParser.java @@ -121,7 +121,7 @@ public class TopHitsParser implements Aggregator.Parser { } factory.scriptFields(scriptFields); } else if (context.parseFieldMatcher().match(currentFieldName, SearchSourceBuilder.HIGHLIGHT_FIELD)) { - factory.highlighter(HighlightBuilder.PROTOTYPE.fromXContent(context)); + factory.highlighter(HighlightBuilder.fromXContent(context)); } else if (context.parseFieldMatcher().match(currentFieldName, SearchSourceBuilder.SORT_FIELD)) { List> sorts = SortBuilder.fromXContent(context); factory.sorts(sorts); diff --git a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index fcb8289849c..8030181e744 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -863,7 +863,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } else if (context.parseFieldMatcher().match(currentFieldName, AGGREGATIONS_FIELD)) { aggregations = aggParsers.parseAggregators(parser, context); } else if (context.parseFieldMatcher().match(currentFieldName, HIGHLIGHT_FIELD)) { - highlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context); + highlightBuilder = HighlightBuilder.fromXContent(context); } else if (context.parseFieldMatcher().match(currentFieldName, INNER_HITS_FIELD)) { innerHitsBuilder = InnerHitsBuilder.fromXContent(parser, context); } else if (context.parseFieldMatcher().match(currentFieldName, SUGGEST_FIELD)) { diff --git a/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java b/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java index 05bc805e1e9..a74b4dabd2b 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryBuilder; @@ -33,11 +34,12 @@ import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.search.highlight.HighlightBuilder.Order; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.BiFunction; + +import static org.elasticsearch.common.xcontent.ObjectParser.fromList; /** * This abstract class holds parameters shared by {@link HighlightBuilder} and {@link HighlightBuilder.Field} @@ -49,7 +51,6 @@ public abstract class AbstractHighlighterBuilder preTagsList = new ArrayList<>(); - while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - preTagsList.add(parser.text()); - } - highlightBuilder.preTags(preTagsList.toArray(new String[preTagsList.size()])); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, POST_TAGS_FIELD)) { - List postTagsList = new ArrayList<>(); - while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - postTagsList.add(parser.text()); - } - highlightBuilder.postTags(postTagsList.toArray(new String[postTagsList.size()])); - } else if (false == highlightBuilder.doFromXContent(parseContext, currentFieldName, token)) { - throw new ParsingException(parser.getTokenLocation(), "cannot parse array with name [{}]", currentFieldName); - } - } else if (token.isValue()) { - if (parseContext.parseFieldMatcher().match(currentFieldName, ORDER_FIELD)) { - highlightBuilder.order(Order.fromString(parser.text())); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, HIGHLIGHT_FILTER_FIELD)) { - highlightBuilder.highlightFilter(parser.booleanValue()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, FRAGMENT_SIZE_FIELD)) { - highlightBuilder.fragmentSize(parser.intValue()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, NUMBER_OF_FRAGMENTS_FIELD)) { - highlightBuilder.numOfFragments(parser.intValue()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, REQUIRE_FIELD_MATCH_FIELD)) { - highlightBuilder.requireFieldMatch(parser.booleanValue()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, BOUNDARY_MAX_SCAN_FIELD)) { - highlightBuilder.boundaryMaxScan(parser.intValue()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, BOUNDARY_CHARS_FIELD)) { - highlightBuilder.boundaryChars(parser.text().toCharArray()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, TYPE_FIELD)) { - highlightBuilder.highlighterType(parser.text()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, FRAGMENTER_FIELD)) { - highlightBuilder.fragmenter(parser.text()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, NO_MATCH_SIZE_FIELD)) { - highlightBuilder.noMatchSize(parser.intValue()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, FORCE_SOURCE_FIELD)) { - highlightBuilder.forceSource(parser.booleanValue()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, PHRASE_LIMIT_FIELD)) { - highlightBuilder.phraseLimit(parser.intValue()); - } else if (false == highlightBuilder.doFromXContent(parseContext, currentFieldName, token)) { - throw new ParsingException(parser.getTokenLocation(), "unexpected fieldname [{}]", currentFieldName); - } - } else if (token == XContentParser.Token.START_OBJECT && currentFieldName != null) { - if (parseContext.parseFieldMatcher().match(currentFieldName, OPTIONS_FIELD)) { - highlightBuilder.options(parser.map()); - } else if (parseContext.parseFieldMatcher().match(currentFieldName, HIGHLIGHT_QUERY_FIELD)) { - highlightBuilder.highlightQuery(parseContext.parseInnerQueryBuilder()); - } else if (false == highlightBuilder.doFromXContent(parseContext, currentFieldName, token)) { - throw new ParsingException(parser.getTokenLocation(), "cannot parse object with name [{}]", currentFieldName); - } - } else if (currentFieldName != null) { - throw new ParsingException(parser.getTokenLocation(), "unexpected token [{}] after [{}]", token, currentFieldName); + static > BiFunction setupParser( + ObjectParser parser) { + parser.declareStringArray(fromList(String.class, HB::preTags), PRE_TAGS_FIELD); + parser.declareStringArray(fromList(String.class, HB::postTags), POST_TAGS_FIELD); + parser.declareString(HB::order, ORDER_FIELD); + parser.declareBoolean(HB::highlightFilter, HIGHLIGHT_FILTER_FIELD); + parser.declareInt(HB::fragmentSize, FRAGMENT_SIZE_FIELD); + parser.declareInt(HB::numOfFragments, NUMBER_OF_FRAGMENTS_FIELD); + parser.declareBoolean(HB::requireFieldMatch, REQUIRE_FIELD_MATCH_FIELD); + parser.declareInt(HB::boundaryMaxScan, BOUNDARY_MAX_SCAN_FIELD); + parser.declareString((HB hb, String bc) -> hb.boundaryChars(bc.toCharArray()) , BOUNDARY_CHARS_FIELD); + parser.declareString(HB::highlighterType, TYPE_FIELD); + parser.declareString(HB::fragmenter, FRAGMENTER_FIELD); + parser.declareInt(HB::noMatchSize, NO_MATCH_SIZE_FIELD); + parser.declareBoolean(HB::forceSource, FORCE_SOURCE_FIELD); + parser.declareInt(HB::phraseLimit, PHRASE_LIMIT_FIELD); + parser.declareObject(HB::options, (XContentParser p, QueryParseContext c) -> { + try { + return p.map(); + } catch (IOException e) { + throw new RuntimeException("Error parsing options", e); } - } - - if (highlightBuilder.preTags() != null && highlightBuilder.postTags() == null) { - throw new ParsingException(parser.getTokenLocation(), "Highlighter global preTags are set, but global postTags are not set"); - } - return highlightBuilder; + }, OPTIONS_FIELD); + parser.declareObject(HB::highlightQuery, (XContentParser p, QueryParseContext c) -> { + try { + return c.parseInnerQueryBuilder(); + } catch (IOException e) { + throw new RuntimeException("Error parsing query", e); + } + }, HIGHLIGHT_QUERY_FIELD); + return (QueryParseContext c, HB hb) -> { + try { + parser.parse(c.parser(), hb, c); + if (hb.preTags() != null && hb.postTags() == null) { + throw new ParsingException(c.parser().getTokenLocation(), + "pre_tags are set but post_tags are not set"); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + return hb; + }; } - /** - * @param parser the input parser. Implementing classes might advance the parser depending on the - * information they need to instantiate a new instance - * @return a new instance - */ - protected abstract HB createInstance(XContentParser parser) throws IOException; - - /** - * Implementing subclasses can handle parsing special options depending on the - * current token, field name and the parse context. - * @return true if an option was found and successfully parsed, otherwise false - */ - protected abstract boolean doFromXContent(QueryParseContext parseContext, String currentFieldName, XContentParser.Token endMarkerToken) throws IOException; - @Override public final int hashCode() { return Objects.hash(getClass(), Arrays.hashCode(preTags), Arrays.hashCode(postTags), fragmentSize, diff --git a/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java b/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java index 514a405b97e..1839a8d160a 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/HighlightBuilder.java @@ -25,9 +25,16 @@ import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; @@ -43,6 +50,9 @@ import java.util.List; import java.util.Locale; import java.util.Objects; import java.util.Set; +import java.util.function.BiFunction; + +import static org.elasticsearch.common.xcontent.ObjectParser.fromList; /** * A builder for search highlighting. Settings can control how large fields @@ -157,6 +167,10 @@ public class HighlightBuilder extends AbstractHighlighterBuilder fields) { + this.fields.addAll(fields); + } + public List fields() { return this.fields; } @@ -217,46 +231,25 @@ public class HighlightBuilder extends AbstractHighlighterBuilder PARSER; + static { + ObjectParser parser = new ObjectParser<>("highlight"); + parser.declareString(HighlightBuilder::tagsSchema, new ParseField("tags_schema")); + parser.declareString(HighlightBuilder::encoder, ENCODER_FIELD); + parser.declareNamedObjects(HighlightBuilder::fields, Field.PARSER, (HighlightBuilder hb) -> hb.useExplicitFieldOrder(true), + FIELDS_FIELD); + PARSER = setupParser(parser); + } + public static HighlightBuilder fromXContent(QueryParseContext c) { + return PARSER.apply(c, new HighlightBuilder()); } public SearchContextHighlight build(QueryShardContext context) throws IOException { @@ -388,11 +381,6 @@ public class HighlightBuilder extends AbstractHighlighterBuilder implements Writeable { static final Field PROTOTYPE = new Field("_na_"); + static final NamedObjectParser PARSER; + static { + ObjectParser parser = new ObjectParser<>("highlight_field"); + parser.declareInt(Field::fragmentOffset, FRAGMENT_OFFSET_FIELD); + parser.declareStringArray(fromList(String.class, Field::matchedFields), MATCHED_FIELDS_FIELD); + BiFunction decoratedParser = setupParser(parser); + PARSER = (XContentParser p, QueryParseContext c, String name) -> decoratedParser.apply(c, new Field(name)); + } private final String name; @@ -476,39 +472,6 @@ public class HighlightBuilder extends AbstractHighlighterBuilder matchedFields = new ArrayList<>(); - while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - matchedFields.add(parser.text()); - } - matchedFields(matchedFields.toArray(new String[matchedFields.size()])); - foundCurrentFieldMatch = true; - } - return foundCurrentFieldMatch; - } - - @Override - protected Field createInstance(XContentParser parser) throws IOException { - if (parser.currentToken() == XContentParser.Token.FIELD_NAME) { - String fieldname = parser.currentName(); - return new Field(fieldname); - } else { - throw new ParsingException(parser.getTokenLocation(), "unknown token type [{}], expected field name", - parser.currentToken()); - } - } - @Override protected int doHashCode() { return Objects.hash(name, fragmentOffset, Arrays.hashCode(matchedFields)); diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java index f8d609c89f9..ec4da62c2dd 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -21,6 +21,8 @@ package org.elasticsearch.common.xcontent; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser.ValueType; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -28,10 +30,17 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import static org.hamcrest.Matchers.hasSize; + public class ObjectParserTests extends ESTestCase { public void testBasics() throws IOException { - XContentParser parser = XContentType.JSON.xContent().createParser("{\"test\" : \"foo\", \"test_number\" : 2, \"testArray\": [1,2,3,4]}"); + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\n" + + " \"test\" : \"foo\",\n" + + " \"test_number\" : 2,\n" + + " \"testArray\": [1,2,3,4]\n" + + "}"); class TestStruct { public String test; int testNumber; @@ -44,7 +53,7 @@ public class ObjectParserTests extends ESTestCase { this.ints = ints; } } - ObjectParser objectParser = new ObjectParser("foo"); + ObjectParser objectParser = new ObjectParser<>("foo"); TestStruct s = new TestStruct(); objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("test"), ObjectParser.ValueType.STRING); @@ -55,29 +64,17 @@ public class ObjectParserTests extends ESTestCase { assertEquals(s.test, "foo"); assertEquals(s.testNumber, 2); assertEquals(s.ints, Arrays.asList(1, 2, 3, 4)); - assertEquals(objectParser.toString(), "ObjectParser{name='foo', fields=[FieldParser{preferred_name=test, supportedTokens=[VALUE_STRING], type=STRING}, FieldParser{preferred_name=test_number, supportedTokens=[VALUE_STRING, VALUE_NUMBER], type=INT}, FieldParser{preferred_name=test_array, supportedTokens=[START_ARRAY, VALUE_STRING, VALUE_NUMBER], type=INT_ARRAY}, FieldParser{preferred_name=test_array, supportedTokens=[START_ARRAY, VALUE_STRING, VALUE_NUMBER], type=INT_ARRAY}, FieldParser{preferred_name=test_number, supportedTokens=[VALUE_STRING, VALUE_NUMBER], type=INT}]}"); - } - - public void testEmptyObject() throws Exception { - XContentParser parser = XContentType.JSON.xContent().createParser("{}"); - class TestStruct { - public String val = null; - public void setVal(String val) { - this.val = val; - } - } - - ObjectParser objectParser = new ObjectParser("eggplant"); - TestStruct s = new TestStruct(); - - objectParser.declareString(TestStruct::setVal, new ParseField("anything")); - objectParser.parse(parser, s); - assertNull("s.val should be null", s.val); + assertEquals(objectParser.toString(), "ObjectParser{name='foo', fields=[" + + "FieldParser{preferred_name=test, supportedTokens=[VALUE_STRING], type=STRING}, " + + "FieldParser{preferred_name=test_number, supportedTokens=[VALUE_STRING, VALUE_NUMBER], type=INT}, " + + "FieldParser{preferred_name=test_array, supportedTokens=[START_ARRAY, VALUE_STRING, VALUE_NUMBER], type=INT_ARRAY}, " + + "FieldParser{preferred_name=test_array, supportedTokens=[START_ARRAY, VALUE_STRING, VALUE_NUMBER], type=INT_ARRAY}, " + + "FieldParser{preferred_name=test_number, supportedTokens=[VALUE_STRING, VALUE_NUMBER], type=INT}]}"); } public void testObjectOrDefault() throws IOException { XContentParser parser = XContentType.JSON.xContent().createParser("{\"object\" : { \"test\": 2}}"); - ObjectParser objectParser = new ObjectParser("foo", StaticTestStruct::new); + ObjectParser objectParser = new ObjectParser<>("foo", StaticTestStruct::new); objectParser.declareInt(StaticTestStruct::setTest, new ParseField("test")); objectParser.declareObjectOrDefault(StaticTestStruct::setObject, objectParser, StaticTestStruct::new, new ParseField("object")); StaticTestStruct s = objectParser.parse(parser); @@ -96,13 +93,10 @@ public class ObjectParserTests extends ESTestCase { public void testExceptions() throws IOException { XContentParser parser = XContentType.JSON.xContent().createParser("{\"test\" : \"foo\"}"); class TestStruct { - public int test; - public void setTest(int test) { - this.test = test; } } - ObjectParser objectParser = new ObjectParser("the_parser"); + ObjectParser objectParser = new ObjectParser<>("the_parser"); TestStruct s = new TestStruct(); objectParser.declareInt(TestStruct::setTest, new ParseField("test")); @@ -128,7 +122,7 @@ public class ObjectParserTests extends ESTestCase { class TestStruct { public String test; } - ObjectParser objectParser = new ObjectParser("foo"); + ObjectParser objectParser = new ObjectParser<>("foo"); TestStruct s = new TestStruct(); objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); @@ -153,7 +147,7 @@ public class ObjectParserTests extends ESTestCase { class TestStruct { public String test; } - ObjectParser objectParser = new ObjectParser("foo"); + ObjectParser objectParser = new ObjectParser<>("foo"); TestStruct s = new TestStruct(); objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("numeric_value"), ObjectParser.ValueType.FLOAT); @@ -172,11 +166,11 @@ public class ObjectParserTests extends ESTestCase { public int test; TestStruct object; } - ObjectParser objectParser = new ObjectParser("foo"); + ObjectParser objectParser = new ObjectParser<>("foo"); TestStruct s = new TestStruct(); s.object = new TestStruct(); - objectParser.declareField((i, c, x) -> c.test = i.intValue(), new ParseField("test"), ObjectParser.ValueType.INT); - objectParser.declareField((i, c, x) -> objectParser.parse(parser, c.object), new ParseField("object"), ObjectParser.ValueType.OBJECT); + objectParser.declareField((i, c, x) -> c.test = i.intValue(), new ParseField("test"), ValueType.INT); + objectParser.declareField((i, c, x) -> objectParser.parse(parser, c.object), new ParseField("object"), ValueType.OBJECT); objectParser.parse(parser, s); assertEquals(s.test, 1); assertEquals(s.object.test, 2); @@ -184,7 +178,7 @@ public class ObjectParserTests extends ESTestCase { public void testParseNestedShortcut() throws IOException { XContentParser parser = XContentType.JSON.xContent().createParser("{ \"test\" : 1, \"object\" : { \"test\": 2}}"); - ObjectParser objectParser = new ObjectParser("foo", StaticTestStruct::new); + ObjectParser objectParser = new ObjectParser<>("foo", StaticTestStruct::new); objectParser.declareInt(StaticTestStruct::setTest, new ParseField("test")); objectParser.declareObject(StaticTestStruct::setObject, objectParser, new ParseField("object")); StaticTestStruct s = objectParser.parse(parser); @@ -192,9 +186,26 @@ public class ObjectParserTests extends ESTestCase { assertEquals(s.object.test, 2); } + public void testEmptyObject() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser("{\"object\" : {}}"); + ObjectParser objectParser = new ObjectParser<>("foo", StaticTestStruct::new); + objectParser.declareObject(StaticTestStruct::setObject, objectParser, new ParseField("object")); + StaticTestStruct s = objectParser.parse(parser); + assertNotNull(s.object); + } + + public void testEmptyObjectInArray() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser("{\"object_array\" : [{}]}"); + ObjectParser objectParser = new ObjectParser<>("foo", StaticTestStruct::new); + objectParser.declareObjectArray(StaticTestStruct::setObjectArray, objectParser, new ParseField("object_array")); + StaticTestStruct s = objectParser.parse(parser); + assertNotNull(s.objectArray); + } + static class StaticTestStruct { - public int test; + int test; StaticTestStruct object; + List objectArray; public void setTest(int test) { this.test = test; @@ -203,6 +214,10 @@ public class ObjectParserTests extends ESTestCase { public void setObject(StaticTestStruct object) { this.object = object; } + + public void setObjectArray(List objectArray) { + this.objectArray = objectArray; + } } enum TestEnum { @@ -218,7 +233,7 @@ public class ObjectParserTests extends ESTestCase { } } XContentParser parser = XContentType.JSON.xContent().createParser("{ \"test\" : \"FOO\" }"); - ObjectParser objectParser = new ObjectParser("foo"); + ObjectParser objectParser = new ObjectParser<>("foo"); objectParser.declareString((struct, value) -> struct.set(TestEnum.valueOf(value)), new ParseField("test")); TestStruct s = objectParser.parse(parser, new TestStruct()); assertEquals(s.test, TestEnum.FOO); @@ -314,7 +329,7 @@ public class ObjectParserTests extends ESTestCase { this.string_or_null = string_or_null; } } - ObjectParser objectParser = new ObjectParser("foo"); + ObjectParser objectParser = new ObjectParser<>("foo"); objectParser.declareInt(TestStruct::setInt_field, new ParseField("int_field")); objectParser.declareIntArray(TestStruct::setInt_array_field, new ParseField("int_array_field")); objectParser.declareLong(TestStruct::setLong_field, new ParseField("long_field")); @@ -352,4 +367,117 @@ public class ObjectParserTests extends ESTestCase { } } + public void testParseNamedObject() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\"named\": {\n" + + " \"a\": {}" + + "}}"); + NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null); + assertThat(h.named, hasSize(1)); + assertEquals("a", h.named.get(0).name); + assertFalse(h.namedSuppliedInOrder); + } + + public void testParseNamedObjectInOrder() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\"named\": [\n" + + " {\"a\": {}}" + + "]}"); + NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null); + assertThat(h.named, hasSize(1)); + assertEquals("a", h.named.get(0).name); + assertTrue(h.namedSuppliedInOrder); + } + + public void testParseNamedObjectTwoFieldsInArray() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\"named\": [\n" + + " {\"a\": {}, \"b\": {}}" + + "]}"); + ParsingException e = expectThrows(ParsingException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); + assertEquals("[named_object_holder] failed to parse field [named]", e.getMessage()); + assertEquals( + "[named] can be a single object with any number of fields or an array where each entry is an object with a single field", + e.getCause().getMessage()); + } + + public void testParseNamedObjectNoFieldsInArray() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\"named\": [\n" + + " {}" + + "]}"); + ParsingException e = expectThrows(ParsingException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); + assertEquals("[named_object_holder] failed to parse field [named]", e.getMessage()); + assertEquals( + "[named] can be a single object with any number of fields or an array where each entry is an object with a single field", + e.getCause().getMessage()); + } + + public void testParseNamedObjectJunkInArray() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\"named\": [\n" + + " \"junk\"" + + "]}"); + ParsingException e = expectThrows(ParsingException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); + assertEquals("[named_object_holder] failed to parse field [named]", e.getMessage()); + assertEquals( + "[named] can be a single object with any number of fields or an array where each entry is an object with a single field", + e.getCause().getMessage()); + } + + public void testParseNamedObjectInOrderNotSupported() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\"named\": [\n" + + " {\"a\": {}}" + + "]}"); + + // Create our own parser for this test so we can disable support for the "ordered" mode specified by the array above + ObjectParser objectParser = new ObjectParser<>("named_object_holder", NamedObjectHolder::new); + objectParser.declareNamedObjects(NamedObjectHolder::setNamed, NamedObject.PARSER, new ParseField("named")); + + // Now firing the xml through it fails + ParsingException e = expectThrows(ParsingException.class, () -> objectParser.apply(parser, null)); + assertEquals("[named_object_holder] failed to parse field [named]", e.getMessage()); + assertEquals("[named] doesn't support arrays. Use a single object with multiple fields.", e.getCause().getMessage()); + } + + static class NamedObjectHolder { + public static final ObjectParser PARSER = new ObjectParser<>("named_object_holder", + NamedObjectHolder::new); + static { + PARSER.declareNamedObjects(NamedObjectHolder::setNamed, NamedObject.PARSER, NamedObjectHolder::keepNamedInOrder, + new ParseField("named")); + } + + private List named; + private boolean namedSuppliedInOrder = false; + + public void setNamed(List named) { + this.named = named; + } + + public void keepNamedInOrder() { + namedSuppliedInOrder = true; + } + } + + public static class NamedObject { + public static final NamedObjectParser PARSER; + static { + ObjectParser parser = new ObjectParser<>("named"); + parser.declareInt(NamedObject::setFoo, new ParseField("foo")); + PARSER = (XContentParser p, Void v, String name) -> parser.parse(p, new NamedObject(name)); + } + + final String name; + int foo; + + public NamedObject(String name) { + this.name = name; + } + + public void setFoo(int foo) { + this.foo = foo; + } + } } diff --git a/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java b/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java index 6cadddfe76a..d885619cf1f 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java @@ -122,14 +122,17 @@ public class HighlightBuilderTests extends ESTestCase { assertTrue("highlighter is not equal to self", secondBuilder.equals(secondBuilder)); assertTrue("highlighter is not equal to its copy", firstBuilder.equals(secondBuilder)); assertTrue("equals is not symmetric", secondBuilder.equals(firstBuilder)); - assertThat("highlighter copy's hashcode is different from original hashcode", secondBuilder.hashCode(), equalTo(firstBuilder.hashCode())); + assertThat("highlighter copy's hashcode is different from original hashcode", secondBuilder.hashCode(), + equalTo(firstBuilder.hashCode())); HighlightBuilder thirdBuilder = serializedCopy(secondBuilder); assertTrue("highlighter is not equal to self", thirdBuilder.equals(thirdBuilder)); assertTrue("highlighter is not equal to its copy", secondBuilder.equals(thirdBuilder)); - assertThat("highlighter copy's hashcode is different from original hashcode", secondBuilder.hashCode(), equalTo(thirdBuilder.hashCode())); + assertThat("highlighter copy's hashcode is different from original hashcode", secondBuilder.hashCode(), + equalTo(thirdBuilder.hashCode())); assertTrue("equals is not transitive", firstBuilder.equals(thirdBuilder)); - assertThat("highlighter copy's hashcode is different from original hashcode", firstBuilder.hashCode(), equalTo(thirdBuilder.hashCode())); + assertThat("highlighter copy's hashcode is different from original hashcode", firstBuilder.hashCode(), + equalTo(thirdBuilder.hashCode())); assertTrue("equals is not symmetric", thirdBuilder.equals(secondBuilder)); assertTrue("equals is not symmetric", thirdBuilder.equals(firstBuilder)); } @@ -152,7 +155,12 @@ public class HighlightBuilderTests extends ESTestCase { XContentParser parser = XContentHelper.createParser(builder.bytes()); context.reset(parser); parser.nextToken(); - HighlightBuilder secondHighlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context); + HighlightBuilder secondHighlightBuilder; + try { + secondHighlightBuilder = HighlightBuilder.fromXContent(context); + } catch (RuntimeException e) { + throw new RuntimeException("Error parsing " + highlightBuilder, e); + } assertNotSame(highlightBuilder, secondHighlightBuilder); assertEquals(highlightBuilder, secondHighlightBuilder); assertEquals(highlightBuilder.hashCode(), secondHighlightBuilder.hashCode()); @@ -163,73 +171,56 @@ public class HighlightBuilderTests extends ESTestCase { * test that unknown array fields cause exception */ public void testUnknownArrayNameExpection() throws IOException { - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); - context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY)); - String highlightElement = "{\n" + - " \"bad_fieldname\" : [ \"field1\" 1 \"field2\" ]\n" + - "}\n"; + { + IllegalArgumentException e = expectParseThrows(IllegalArgumentException.class, "{\n" + + " \"bad_fieldname\" : [ \"field1\" 1 \"field2\" ]\n" + + "}\n"); + assertEquals("[highlight] unknown field [bad_fieldname], parser not found", e.getMessage()); + } + + { + ParsingException e = expectParseThrows(ParsingException.class, "{\n" + + " \"fields\" : {\n" + + " \"body\" : {\n" + + " \"bad_fieldname\" : [ \"field1\" , \"field2\" ]\n" + + " }\n" + + " }\n" + + "}\n"); + assertEquals("[highlight] failed to parse field [fields]", e.getMessage()); + assertEquals("[fields] failed to parse field [body]", e.getCause().getMessage()); + assertEquals("[highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage()); + } + } + + private T expectParseThrows(Class exceptionClass, String highlightElement) throws IOException { XContentParser parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); - + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); context.reset(parser); - try { - HighlightBuilder.PROTOTYPE.fromXContent(context); - fail("expected a parsing exception"); - } catch (ParsingException e) { - assertEquals("cannot parse array with name [bad_fieldname]", e.getMessage()); - } - - highlightElement = "{\n" + - " \"fields\" : {\n" + - " \"body\" : {\n" + - " \"bad_fieldname\" : [ \"field1\" , \"field2\" ]\n" + - " }\n" + - " }\n" + - "}\n"; - parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); - - context.reset(parser); - try { - HighlightBuilder.PROTOTYPE.fromXContent(context); - fail("expected a parsing exception"); - } catch (ParsingException e) { - assertEquals("cannot parse array with name [bad_fieldname]", e.getMessage()); - } + return expectThrows(exceptionClass, () -> HighlightBuilder.fromXContent(context)); } /** * test that unknown field name cause exception */ public void testUnknownFieldnameExpection() throws IOException { - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); - context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY)); - String highlightElement = "{\n" + - " \"bad_fieldname\" : \"value\"\n" + - "}\n"; - XContentParser parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); - - context.reset(parser); - try { - HighlightBuilder.PROTOTYPE.fromXContent(context); - fail("expected a parsing exception"); - } catch (ParsingException e) { - assertEquals("unexpected fieldname [bad_fieldname]", e.getMessage()); + { + IllegalArgumentException e = expectParseThrows(IllegalArgumentException.class, "{\n" + + " \"bad_fieldname\" : \"value\"\n" + + "}\n"); + assertEquals("[highlight] unknown field [bad_fieldname], parser not found", e.getMessage()); } - highlightElement = "{\n" + - " \"fields\" : {\n" + - " \"body\" : {\n" + - " \"bad_fieldname\" : \"value\"\n" + - " }\n" + - " }\n" + - "}\n"; - parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); - - context.reset(parser); - try { - HighlightBuilder.PROTOTYPE.fromXContent(context); - fail("expected a parsing exception"); - } catch (ParsingException e) { - assertEquals("unexpected fieldname [bad_fieldname]", e.getMessage()); + { + ParsingException e = expectParseThrows(ParsingException.class, "{\n" + + " \"fields\" : {\n" + + " \"body\" : {\n" + + " \"bad_fieldname\" : \"value\"\n" + + " }\n" + + " }\n" + + "}\n"); + assertEquals("[highlight] failed to parse field [fields]", e.getMessage()); + assertEquals("[fields] failed to parse field [body]", e.getCause().getMessage()); + assertEquals("[highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage()); } } @@ -237,38 +228,57 @@ public class HighlightBuilderTests extends ESTestCase { * test that unknown field name cause exception */ public void testUnknownObjectFieldnameExpection() throws IOException { - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); - context.parseFieldMatcher(new ParseFieldMatcher(Settings.EMPTY)); - String highlightElement = "{\n" + - " \"bad_fieldname\" : { \"field\" : \"value\" }\n \n" + - "}\n"; - XContentParser parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); - - context.reset(parser); - try { - HighlightBuilder.PROTOTYPE.fromXContent(context); - fail("expected a parsing exception"); - } catch (ParsingException e) { - assertEquals("cannot parse object with name [bad_fieldname]", e.getMessage()); + { + IllegalArgumentException e = expectParseThrows(IllegalArgumentException.class, "{\n" + + " \"bad_fieldname\" : { \"field\" : \"value\" }\n \n" + + "}\n"); + assertEquals("[highlight] unknown field [bad_fieldname], parser not found", e.getMessage()); } - highlightElement = "{\n" + - " \"fields\" : {\n" + - " \"body\" : {\n" + - " \"bad_fieldname\" : { \"field\" : \"value\" }\n" + - " }\n" + - " }\n" + - "}\n"; - parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); - - context.reset(parser); - try { - HighlightBuilder.PROTOTYPE.fromXContent(context); - fail("expected a parsing exception"); - } catch (ParsingException e) { - assertEquals("cannot parse object with name [bad_fieldname]", e.getMessage()); + { + ParsingException e = expectParseThrows(ParsingException.class, "{\n" + + " \"fields\" : {\n" + + " \"body\" : {\n" + + " \"bad_fieldname\" : { \"field\" : \"value\" }\n" + + " }\n" + + " }\n" + + "}\n"); + assertEquals("[highlight] failed to parse field [fields]", e.getMessage()); + assertEquals("[fields] failed to parse field [body]", e.getCause().getMessage()); + assertEquals("[highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage()); } - } + } + + public void testStringInFieldsArray() throws IOException { + ParsingException e = expectParseThrows(ParsingException.class, "{\"fields\" : [ \"junk\" ]}"); + assertEquals("[highlight] failed to parse field [fields]", e.getMessage()); + assertEquals( + "[fields] can be a single object with any number of fields or an array where each entry is an object with a single field", + e.getCause().getMessage()); + } + + public void testNoFieldsInObjectInFieldsArray() throws IOException { + ParsingException e = expectParseThrows(ParsingException.class, "{\n" + + " \"fields\" : [ {\n" + + " }] \n" + + "}\n"); + assertEquals("[highlight] failed to parse field [fields]", e.getMessage()); + assertEquals( + "[fields] can be a single object with any number of fields or an array where each entry is an object with a single field", + e.getCause().getMessage()); + } + + public void testTwoFieldsInObjectInFieldsArray() throws IOException { + ParsingException e = expectParseThrows(ParsingException.class, "{\n" + + " \"fields\" : [ {\n" + + " \"body\" : {},\n" + + " \"nope\" : {}\n" + + " }] \n" + + "}\n"); + assertEquals("[highlight] failed to parse field [fields]", e.getMessage()); + assertEquals( + "[fields] can be a single object with any number of fields or an array where each entry is an object with a single field", + e.getCause().getMessage()); } /** * test that build() outputs a {@link SearchContextHighlight} that is has similar parameters @@ -280,7 +290,8 @@ public class HighlightBuilderTests extends ESTestCase { Index index = new Index(randomAsciiOfLengthBetween(1, 10), "_na_"); 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, indicesQueriesRegistry, null) { + QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, indicesQueriesRegistry, + null) { @Override public MappedFieldType fieldMapper(String name) { TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name); @@ -396,7 +407,7 @@ public class HighlightBuilderTests extends ESTestCase { XContentParser parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); context.reset(parser); - HighlightBuilder highlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context); + HighlightBuilder highlightBuilder = HighlightBuilder.fromXContent(context); 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.DEFAULT_STYLED_POST_TAGS, @@ -408,24 +419,17 @@ public class HighlightBuilderTests extends ESTestCase { parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); context.reset(parser); - highlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context); + highlightBuilder = HighlightBuilder.fromXContent(context); assertArrayEquals("setting tags_schema 'default' should alter pre_tags", HighlightBuilder.DEFAULT_PRE_TAGS, highlightBuilder.preTags()); assertArrayEquals("setting tags_schema 'default' should alter post_tags", HighlightBuilder.DEFAULT_POST_TAGS, highlightBuilder.postTags()); - highlightElement = "{\n" + + ParsingException e = expectParseThrows(ParsingException.class, "{\n" + " \"tags_schema\" : \"somthing_else\"\n" + - "}\n"; - parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); - - context.reset(parser); - try { - HighlightBuilder.PROTOTYPE.fromXContent(context); - fail("setting unknown tag schema should throw exception"); - } catch (IllegalArgumentException e) { - assertEquals("Unknown tag schema [somthing_else]", e.getMessage()); - } + "}\n"); + assertEquals("[highlight] failed to parse field [tags_schema]", e.getMessage()); + assertEquals("Unknown tag schema [somthing_else]", e.getCause().getMessage()); } /** @@ -438,24 +442,42 @@ public class HighlightBuilderTests extends ESTestCase { XContentParser parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); context.reset(parser); - HighlightBuilder highlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context); + HighlightBuilder highlightBuilder = HighlightBuilder.fromXContent(context); assertEquals("expected plain HighlightBuilder", new HighlightBuilder(), highlightBuilder); highlightElement = "{ \"fields\" : { } }"; parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); context.reset(parser); - highlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context); + highlightBuilder = HighlightBuilder.fromXContent(context); assertEquals("defining no field should return plain HighlightBuilder", new HighlightBuilder(), highlightBuilder); highlightElement = "{ \"fields\" : { \"foo\" : { } } }"; parser = XContentFactory.xContent(highlightElement).createParser(highlightElement); context.reset(parser); - highlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context); + highlightBuilder = HighlightBuilder.fromXContent(context); assertEquals("expected HighlightBuilder with field", new HighlightBuilder().field(new Field("foo")), highlightBuilder); } + public void testPreTagsWithoutPostTags() throws IOException { + ParsingException e = expectParseThrows(ParsingException.class, "{\n" + + " \"pre_tags\" : [\"\"]\n" + + "}\n"); + assertEquals("pre_tags are set but post_tags are not set", e.getMessage()); + + e = expectParseThrows(ParsingException.class, "{\n" + + " \"fields\" : {\n" + + " \"body\" : {\n" + + " \"pre_tags\" : [\"\"]\n" + + " }\n" + + " }\n" + + "}\n"); + assertEquals("[highlight] failed to parse field [fields]", e.getMessage()); + assertEquals("[fields] failed to parse field [body]", e.getCause().getMessage()); + assertEquals("pre_tags are set but post_tags are not set", e.getCause().getCause().getMessage()); + } + /** * test ordinals of {@link Order}, since serialization depends on it */