1
0
Эх сурвалжийг харах

Enable 'encoder' and 'tags_schema' highlighting settings at field level (#107224)

Executing a search query with highlighting and specifying 'encoder' or 'tags_schema' settings at the field level was resulting in an x_content_parse_exception.

This PR moves 'encoder' and 'tags_schema' highlighting setting on AbstractHighlighterBuilder, allowing them to be set global and at field level.
Lefteris Katiforis 1 жил өмнө
parent
commit
c8c918758c

+ 6 - 0
docs/changelog/107224.yaml

@@ -0,0 +1,6 @@
+pr: 107224
+summary: "Enable 'encoder' and 'tags_schema' highlighting settings at field level"
+area: Highlighting
+type: enhancement
+issues:
+  - 94028

+ 1 - 0
server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -170,6 +170,7 @@ public class TransportVersions {
     public static final TransportVersion ML_INFERENCE_TIMEOUT_ADDED = def(8_629_00_0);
     public static final TransportVersion MODIFY_DATA_STREAM_FAILURE_STORES = def(8_630_00_0);
     public static final TransportVersion ML_INFERENCE_RERANK_NEW_RESPONSE_FORMAT = def(8_631_00_0);
+    public static final TransportVersion HIGHLIGHTERS_TAGS_ON_FIELD_LEVEL = def(8_632_00_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

+ 55 - 0
server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/AbstractHighlighterBuilder.java

@@ -54,6 +54,7 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
     public static final ParseField FRAGMENT_OFFSET_FIELD = new ParseField("fragment_offset");
     public static final ParseField NUMBER_OF_FRAGMENTS_FIELD = new ParseField("number_of_fragments");
     public static final ParseField ENCODER_FIELD = new ParseField("encoder");
+    public static final ParseField TAGS_SCHEMA_FIELD = new ParseField("tags_schema");
     public static final ParseField REQUIRE_FIELD_MATCH_FIELD = new ParseField("require_field_match");
     public static final ParseField BOUNDARY_SCANNER_FIELD = new ParseField("boundary_scanner");
     public static final ParseField BOUNDARY_MAX_SCAN_FIELD = new ParseField("boundary_max_scan");
@@ -69,6 +70,8 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
     public static final ParseField MATCHED_FIELDS_FIELD = new ParseField("matched_fields");
     public static final ParseField MAX_ANALYZED_OFFSET_FIELD = new ParseField("max_analyzed_offset");
 
+    protected String encoder;
+
     protected String[] preTags;
 
     protected String[] postTags;
@@ -112,6 +115,7 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
         postTags = template.postTags;
         fragmentSize = template.fragmentSize;
         numOfFragments = template.numOfFragments;
+        encoder = template.encoder;
         highlighterType = template.highlighterType;
         fragmenter = template.fragmenter;
         highlightQuery = queryBuilder;
@@ -137,6 +141,9 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
         postTags(in.readOptionalStringArray());
         fragmentSize(in.readOptionalVInt());
         numOfFragments(in.readOptionalVInt());
+        if (in.getTransportVersion().onOrAfter(TransportVersions.HIGHLIGHTERS_TAGS_ON_FIELD_LEVEL)) {
+            encoder(in.readOptionalString());
+        }
         highlighterType(in.readOptionalString());
         fragmenter(in.readOptionalString());
         if (in.readBoolean()) {
@@ -173,6 +180,9 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
         out.writeOptionalStringArray(postTags);
         out.writeOptionalVInt(fragmentSize);
         out.writeOptionalVInt(numOfFragments);
+        if (out.getTransportVersion().onOrAfter(TransportVersions.HIGHLIGHTERS_TAGS_ON_FIELD_LEVEL)) {
+            out.writeOptionalString(encoder);
+        }
         out.writeOptionalString(highlighterType);
         out.writeOptionalString(fragmenter);
         boolean hasQuery = highlightQuery != null;
@@ -275,6 +285,44 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
         return this.numOfFragments;
     }
 
+    /**
+     * Set the encoder, defaults to {@link HighlightBuilder#DEFAULT_ENCODER}
+     */
+    @SuppressWarnings("unchecked")
+    public HB encoder(String encoder) {
+        this.encoder = encoder;
+        return (HB) this;
+    }
+
+    /**
+     * @return the value set by {@link #encoder(String)}
+     */
+    public String encoder() {
+        return this.encoder;
+    }
+
+    /**
+     * Set a tag scheme that encapsulates a built in pre and post tags. The allowed schemes
+     * are {@code styled} and {@code default}.
+     *
+     * @param schemaName The tag scheme name
+     */
+    @SuppressWarnings("unchecked")
+    public HB tagsSchema(String schemaName) {
+        switch (schemaName) {
+            case "default" -> {
+                preTags(HighlightBuilder.DEFAULT_PRE_TAGS);
+                postTags(HighlightBuilder.DEFAULT_POST_TAGS);
+            }
+            case "styled" -> {
+                preTags(HighlightBuilder.DEFAULT_STYLED_PRE_TAG);
+                postTags(HighlightBuilder.DEFAULT_STYLED_POST_TAGS);
+            }
+            default -> throw new IllegalArgumentException("Unknown tag schema [" + schemaName + "]");
+        }
+        return (HB) this;
+    }
+
     /**
      * Set type of highlighter to use. Out of the box supported types
      * are {@code unified}, {@code plain} and {@code fvh}.
@@ -561,6 +609,9 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
         if (numOfFragments != null) {
             builder.field(NUMBER_OF_FRAGMENTS_FIELD.getPreferredName(), numOfFragments);
         }
+        if (encoder != null) {
+            builder.field(ENCODER_FIELD.getPreferredName(), encoder);
+        }
         if (highlighterType != null) {
             builder.field(TYPE_FIELD.getPreferredName(), highlighterType);
         }
@@ -612,6 +663,8 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
         parser.declareBoolean(HB::highlightFilter, HIGHLIGHT_FILTER_FIELD);
         parser.declareInt(HB::fragmentSize, FRAGMENT_SIZE_FIELD);
         parser.declareInt(HB::numOfFragments, NUMBER_OF_FRAGMENTS_FIELD);
+        parser.declareString(HB::encoder, ENCODER_FIELD);
+        parser.declareString(HB::tagsSchema, TAGS_SCHEMA_FIELD);
         parser.declareBoolean(HB::requireFieldMatch, REQUIRE_FIELD_MATCH_FIELD);
         parser.declareString(HB::boundaryScannerType, BOUNDARY_SCANNER_FIELD);
         parser.declareInt(HB::boundaryMaxScan, BOUNDARY_MAX_SCAN_FIELD);
@@ -661,6 +714,7 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
             Arrays.hashCode(postTags),
             fragmentSize,
             numOfFragments,
+            encoder,
             highlighterType,
             fragmenter,
             highlightQuery,
@@ -698,6 +752,7 @@ public abstract class AbstractHighlighterBuilder<HB extends AbstractHighlighterB
             && Arrays.equals(postTags, other.postTags)
             && Objects.equals(fragmentSize, other.fragmentSize)
             && Objects.equals(numOfFragments, other.numOfFragments)
+            && Objects.equals(encoder, other.encoder)
             && Objects.equals(highlighterType, other.highlighterType)
             && Objects.equals(fragmenter, other.fragmenter)
             && Objects.equals(highlightQuery, other.highlightQuery)

+ 13 - 55
server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilder.java

@@ -10,6 +10,7 @@ package org.elasticsearch.search.fetch.subphase.highlight;
 
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.vectorhighlight.SimpleBoundaryScanner;
+import org.elasticsearch.TransportVersions;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -21,7 +22,6 @@ import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.search.fetch.subphase.highlight.SearchHighlightContext.FieldOptions;
 import org.elasticsearch.xcontent.ObjectParser;
 import org.elasticsearch.xcontent.ObjectParser.NamedObjectParser;
-import org.elasticsearch.xcontent.ParseField;
 import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentParser;
 
@@ -104,8 +104,6 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
 
     private final List<Field> fields;
 
-    private String encoder;
-
     private boolean useExplicitFieldOrder = false;
 
     public HighlightBuilder() {
@@ -114,7 +112,6 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
 
     public HighlightBuilder(HighlightBuilder template, QueryBuilder highlightQuery, List<Field> fields) {
         super(template, highlightQuery);
-        this.encoder = template.encoder;
         this.useExplicitFieldOrder = template.useExplicitFieldOrder;
         this.fields = fields;
     }
@@ -124,7 +121,9 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
      */
     public HighlightBuilder(StreamInput in) throws IOException {
         super(in);
-        encoder(in.readOptionalString());
+        if (in.getTransportVersion().before(TransportVersions.HIGHLIGHTERS_TAGS_ON_FIELD_LEVEL)) {
+            encoder(in.readOptionalString());
+        }
         useExplicitFieldOrder(in.readBoolean());
         this.fields = in.readCollectionAsList(Field::new);
         assert this.equals(new HighlightBuilder(this, highlightQuery, fields)) : "copy constructor is broken";
@@ -132,7 +131,9 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
 
     @Override
     protected void doWriteTo(StreamOutput out) throws IOException {
-        out.writeOptionalString(encoder);
+        if (out.getTransportVersion().before(TransportVersions.HIGHLIGHTERS_TAGS_ON_FIELD_LEVEL)) {
+            out.writeOptionalString(encoder);
+        }
         out.writeBoolean(useExplicitFieldOrder);
         out.writeCollection(fields);
     }
@@ -185,45 +186,6 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
         return this.fields;
     }
 
-    /**
-     * Set a tag scheme that encapsulates a built in pre and post tags. The allowed schemes
-     * are {@code styled} and {@code default}.
-     *
-     * @param schemaName The tag scheme name
-     */
-    public HighlightBuilder tagsSchema(String schemaName) {
-        switch (schemaName) {
-            case "default" -> {
-                preTags(DEFAULT_PRE_TAGS);
-                postTags(DEFAULT_POST_TAGS);
-            }
-            case "styled" -> {
-                preTags(DEFAULT_STYLED_PRE_TAG);
-                postTags(DEFAULT_STYLED_POST_TAGS);
-            }
-            default -> throw new IllegalArgumentException("Unknown tag schema [" + schemaName + "]");
-        }
-        return this;
-    }
-
-    /**
-     * Set encoder for the highlighting
-     * are {@code html} and {@code default}.
-     *
-     * @param encoder name
-     */
-    public HighlightBuilder encoder(String encoder) {
-        this.encoder = encoder;
-        return this;
-    }
-
-    /**
-     * Getter for {@link #encoder(String)}
-     */
-    public String encoder() {
-        return this.encoder;
-    }
-
     /**
      * Send the fields to be highlighted using a syntax that is specific about the order in which they should be highlighted.
      * @return this for chaining
@@ -251,8 +213,6 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
     private static final BiFunction<XContentParser, HighlightBuilder, HighlightBuilder> PARSER;
     static {
         ObjectParser<HighlightBuilder, Void> parser = new ObjectParser<>("highlight");
-        parser.declareString(HighlightBuilder::tagsSchema, new ParseField("tags_schema"));
-        parser.declareString(HighlightBuilder::encoder, ENCODER_FIELD);
         parser.declareNamedObjects(
             HighlightBuilder::fields,
             Field.PARSER,
@@ -269,7 +229,7 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
     public SearchHighlightContext build(SearchExecutionContext context) throws IOException {
         // create template global options that are later merged with any partial field options
         final SearchHighlightContext.FieldOptions.Builder globalOptionsBuilder = new SearchHighlightContext.FieldOptions.Builder();
-        globalOptionsBuilder.encoder(this.encoder);
+
         transferOptions(this, globalOptionsBuilder, context);
 
         // overwrite unset global options by default values
@@ -325,6 +285,9 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
         if (highlighterBuilder.numOfFragments != null) {
             targetOptionsBuilder.numberOfFragments(highlighterBuilder.numOfFragments);
         }
+        if (highlighterBuilder.encoder != null) {
+            targetOptionsBuilder.encoder(highlighterBuilder.encoder);
+        }
         if (highlighterBuilder.requireFieldMatch != null) {
             targetOptionsBuilder.requireFieldMatch(highlighterBuilder.requireFieldMatch);
         }
@@ -379,9 +342,6 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
         // first write common options
         commonOptionsToXContent(builder);
         // special options for top-level highlighter
-        if (encoder != null) {
-            builder.field(ENCODER_FIELD.getPreferredName(), encoder);
-        }
         if (fields.size() > 0) {
             if (useExplicitFieldOrder) {
                 builder.startArray(FIELDS_FIELD.getPreferredName());
@@ -407,14 +367,12 @@ public final class HighlightBuilder extends AbstractHighlighterBuilder<Highlight
 
     @Override
     protected int doHashCode() {
-        return Objects.hash(encoder, useExplicitFieldOrder, fields);
+        return Objects.hash(useExplicitFieldOrder, fields);
     }
 
     @Override
     protected boolean doEquals(HighlightBuilder other) {
-        return Objects.equals(encoder, other.encoder)
-            && Objects.equals(useExplicitFieldOrder, other.useExplicitFieldOrder)
-            && Objects.equals(fields, other.fields);
+        return Objects.equals(useExplicitFieldOrder, other.useExplicitFieldOrder) && Objects.equals(fields, other.fields);
     }
 
     @Override

+ 47 - 5
server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java

@@ -332,8 +332,6 @@ public class HighlightBuilderTests extends ESTestCase {
             highlightBuilder = Rewriteable.rewrite(highlightBuilder, mockContext);
             SearchHighlightContext highlight = highlightBuilder.build(mockContext);
             for (SearchHighlightContext.Field field : highlight.fields()) {
-                String encoder = highlightBuilder.encoder() != null ? highlightBuilder.encoder() : HighlightBuilder.DEFAULT_ENCODER;
-                assertEquals(encoder, field.fieldOptions().encoder());
                 final Field fieldBuilder = getFieldBuilderByName(highlightBuilder, field.field());
                 assertNotNull("expected a highlight builder for field " + field.field(), fieldBuilder);
                 FieldOptions fieldOptions = field.fieldOptions();
@@ -344,6 +342,7 @@ public class HighlightBuilderTests extends ESTestCase {
                     fieldOptions
                 );
 
+                checkSame.accept(AbstractHighlighterBuilder::encoder, FieldOptions::encoder);
                 checkSame.accept(AbstractHighlighterBuilder::boundaryChars, FieldOptions::boundaryChars);
                 checkSame.accept(AbstractHighlighterBuilder::boundaryScannerType, FieldOptions::boundaryScannerType);
                 checkSame.accept(AbstractHighlighterBuilder::boundaryMaxScan, FieldOptions::boundaryMaxScan);
@@ -477,6 +476,49 @@ public class HighlightBuilderTests extends ESTestCase {
                 highlightBuilder.postTags()
             );
 
+        }
+
+        highlightElement = """
+            {
+                "fields": {
+                    "default_string": {
+                        "tags_schema": "default"
+                    },
+                    "styled_string": {
+                        "tags_schema": "styled"
+                    }
+                }
+            }
+            """;
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, highlightElement)) {
+
+            HighlightBuilder highlightBuilder = HighlightBuilder.fromXContent(parser);
+            Field defaultStringField = getFieldBuilderByName(highlightBuilder, "default_string");
+            assertNotNull(defaultStringField);
+            assertArrayEquals(
+                "setting tags_schema 'default' at the field level should alter pre_tags for that field",
+                HighlightBuilder.DEFAULT_PRE_TAGS,
+                defaultStringField.preTags()
+            );
+            assertArrayEquals(
+                "setting tags_schema 'default' at the field level should alter post_tags for that that field",
+                HighlightBuilder.DEFAULT_POST_TAGS,
+                defaultStringField.postTags()
+            );
+
+            Field styledStringField = getFieldBuilderByName(highlightBuilder, "styled_string");
+            assertNotNull(styledStringField);
+            assertArrayEquals(
+                "setting tags_schema 'styled' at the field level should alter pre_tags for that that field",
+                HighlightBuilder.DEFAULT_STYLED_PRE_TAG,
+                styledStringField.preTags()
+            );
+            assertArrayEquals(
+                "setting tags_schema 'styled' at the field level should alter post_tags for that that field",
+                HighlightBuilder.DEFAULT_STYLED_POST_TAGS,
+                styledStringField.postTags()
+            );
+
             XContentParseException e = expectParseThrows(XContentParseException.class, """
                 {
                     "tags_schema" : "somthing_else"
@@ -592,9 +634,6 @@ public class HighlightBuilderTests extends ESTestCase {
         HighlightBuilder testHighlighter = new HighlightBuilder();
         setRandomCommonOptions(testHighlighter);
         testHighlighter.useExplicitFieldOrder(randomBoolean());
-        if (randomBoolean()) {
-            testHighlighter.encoder(randomFrom(Arrays.asList(new String[] { "default", "html" })));
-        }
         int numberOfFields = randomIntBetween(1, 5);
         for (int i = 0; i < numberOfFields; i++) {
             Field field = new Field(i + "_" + randomAlphaOfLengthBetween(1, 10));
@@ -617,6 +656,9 @@ public class HighlightBuilderTests extends ESTestCase {
             highlightBuilder.preTags(randomStringArray(1, 3));
             highlightBuilder.postTags(randomStringArray(1, 3));
         }
+        if (randomBoolean()) {
+            highlightBuilder.encoder(randomFrom(Arrays.asList("default", "html")));
+        }
         if (randomBoolean()) {
             highlightBuilder.fragmentSize(randomIntBetween(0, 100));
         }