From 336453e62232059d101903c138457bf7fab28933 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 11 May 2023 16:48:26 +0530 Subject: [PATCH] Remove builder instance from TextFieldMapper (#7159) (#7519) * Remove builder instance from TextFieldMapper * Add test coverage for mapping params serialization (cherry picked from commit dffd822a57289ac0c670ac2a0ff41e30eb8f08de) Signed-off-by: Sooraj Sinha Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../index/mapper/TextFieldMapper.java | 94 ++++++++++++------- .../index/mapper/TextFieldMapperTests.java | 38 +++++++- 2 files changed, 96 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/TextFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/TextFieldMapper.java index 825d95bd5e42f..a570c1c473cfd 100644 --- a/server/src/main/java/org/opensearch/index/mapper/TextFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/TextFieldMapper.java @@ -32,6 +32,7 @@ package org.opensearch.index.mapper; +import java.util.Optional; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.AnalyzerWrapper; import org.apache.lucene.analysis.CachingTokenFilter; @@ -143,8 +144,8 @@ public static class Defaults { public static final int POSITION_INCREMENT_GAP = 100; } - private static Builder builder(FieldMapper in) { - return ((TextFieldMapper) in).builder; + private static TextFieldMapper toType(FieldMapper in) { + return (TextFieldMapper) in; } /** @@ -280,44 +281,56 @@ public static class Builder extends ParametrizedFieldMapper.Builder { private final Version indexCreatedVersion; - private final Parameter index = Parameter.indexParam(m -> builder(m).index.getValue(), true); - private final Parameter store = Parameter.storeParam(m -> builder(m).store.getValue(), false); + private final Parameter index = Parameter.indexParam(m -> toType(m).mappedFieldType.isSearchable(), true); + private final Parameter store = Parameter.storeParam(m -> toType(m).fieldType.stored(), false); - final Parameter similarity = TextParams.similarity(m -> builder(m).similarity.getValue()); + final Parameter similarity = TextParams.similarity(m -> toType(m).similarity); - final Parameter indexOptions = TextParams.indexOptions(m -> builder(m).indexOptions.getValue()); - final Parameter norms = TextParams.norms(true, m -> builder(m).norms.getValue()); - final Parameter termVectors = TextParams.termVectors(m -> builder(m).termVectors.getValue()); + final Parameter indexOptions = TextParams.indexOptions(m -> toType(m).indexOptions); + final Parameter norms = TextParams.norms(true, m -> toType(m).fieldType.omitNorms() == false); + final Parameter termVectors = TextParams.termVectors(m -> toType(m).termVectors); final Parameter positionIncrementGap = Parameter.intParam( "position_increment_gap", false, - m -> builder(m).positionIncrementGap.getValue(), + m -> toType(m).positionIncrementGap, POSITION_INCREMENT_GAP_USE_ANALYZER ); - final Parameter fieldData = Parameter.boolParam("fielddata", true, m -> builder(m).fieldData.getValue(), false); + final Parameter fieldData = Parameter.boolParam( + "fielddata", + true, + m -> ((TextFieldType) toType(m).mappedFieldType).fielddata, + false + ); final Parameter freqFilter = new Parameter<>( "fielddata_frequency_filter", true, () -> DEFAULT_FILTER, TextFieldMapper::parseFrequencyFilter, - m -> builder(m).freqFilter.getValue() + m -> toType(m).freqFilter ); final Parameter eagerGlobalOrdinals = Parameter.boolParam( "eager_global_ordinals", true, - m -> builder(m).eagerGlobalOrdinals.getValue(), + m -> toType(m).mappedFieldType.eagerGlobalOrdinals(), false ); - final Parameter indexPhrases = Parameter.boolParam("index_phrases", false, m -> builder(m).indexPhrases.getValue(), false); + final Parameter indexPhrases = Parameter.boolParam( + "index_phrases", + false, + m -> ((TextFieldType) toType(m).mappedFieldType).indexPhrases, + false + ); final Parameter indexPrefixes = new Parameter<>( "index_prefixes", false, () -> null, TextFieldMapper::parsePrefixConfig, - m -> builder(m).indexPrefixes.getValue() + m -> Optional.ofNullable(((TextFieldType) toType(m).mappedFieldType).prefixFieldType) + .map(p -> new PrefixConfig(p.minChars, p.maxChars)) + .orElse(null) ).acceptsNull(); private final Parameter boost = Parameter.boostParam(); @@ -956,10 +969,16 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S } - private final Builder builder; private final FieldType fieldType; private final PrefixFieldMapper prefixFieldMapper; private final PhraseFieldMapper phraseFieldMapper; + private final SimilarityProvider similarity; + private final String indexOptions; + private final String termVectors; + private final int positionIncrementGap; + private final Version indexCreatedVersion; + private final IndexAnalyzers indexAnalyzers; + private final FielddataFrequencyFilter freqFilter; protected TextFieldMapper( String simpleName, @@ -980,7 +999,13 @@ protected TextFieldMapper( this.fieldType = fieldType; this.prefixFieldMapper = prefixFieldMapper; this.phraseFieldMapper = phraseFieldMapper; - this.builder = builder; + this.similarity = builder.similarity.getValue(); + this.indexOptions = builder.indexOptions.getValue(); + this.termVectors = builder.termVectors.getValue(); + this.positionIncrementGap = builder.positionIncrementGap.getValue(); + this.indexCreatedVersion = builder.indexCreatedVersion; + this.indexAnalyzers = builder.analyzers.indexAnalyzers; + this.freqFilter = builder.freqFilter.getValue(); } @Override @@ -990,7 +1015,7 @@ protected TextFieldMapper clone() { @Override public ParametrizedFieldMapper.Builder getMergeBuilder() { - return new Builder(simpleName(), builder.indexCreatedVersion, builder.analyzers.indexAnalyzers).init(this); + return new Builder(simpleName(), this.indexCreatedVersion, this.indexAnalyzers).init(this); } @Override @@ -1169,24 +1194,25 @@ public static Query createPhrasePrefixQuery( protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException { // this is a pain, but we have to do this to maintain BWC builder.field("type", contentType()); - this.builder.boost.toXContent(builder, includeDefaults); - this.builder.index.toXContent(builder, includeDefaults); - this.builder.store.toXContent(builder, includeDefaults); + Builder mapperBuilder = (TextFieldMapper.Builder) getMergeBuilder(); + mapperBuilder.boost.toXContent(builder, includeDefaults); + mapperBuilder.index.toXContent(builder, includeDefaults); + mapperBuilder.store.toXContent(builder, includeDefaults); this.multiFields.toXContent(builder, params); this.copyTo.toXContent(builder, params); - this.builder.meta.toXContent(builder, includeDefaults); - this.builder.indexOptions.toXContent(builder, includeDefaults); - this.builder.termVectors.toXContent(builder, includeDefaults); - this.builder.norms.toXContent(builder, includeDefaults); - this.builder.analyzers.indexAnalyzer.toXContent(builder, includeDefaults); - this.builder.analyzers.searchAnalyzer.toXContent(builder, includeDefaults); - this.builder.analyzers.searchQuoteAnalyzer.toXContent(builder, includeDefaults); - this.builder.similarity.toXContent(builder, includeDefaults); - this.builder.eagerGlobalOrdinals.toXContent(builder, includeDefaults); - this.builder.positionIncrementGap.toXContent(builder, includeDefaults); - this.builder.fieldData.toXContent(builder, includeDefaults); - this.builder.freqFilter.toXContent(builder, includeDefaults); - this.builder.indexPrefixes.toXContent(builder, includeDefaults); - this.builder.indexPhrases.toXContent(builder, includeDefaults); + mapperBuilder.meta.toXContent(builder, includeDefaults); + mapperBuilder.indexOptions.toXContent(builder, includeDefaults); + mapperBuilder.termVectors.toXContent(builder, includeDefaults); + mapperBuilder.norms.toXContent(builder, includeDefaults); + mapperBuilder.analyzers.indexAnalyzer.toXContent(builder, includeDefaults); + mapperBuilder.analyzers.searchAnalyzer.toXContent(builder, includeDefaults); + mapperBuilder.analyzers.searchQuoteAnalyzer.toXContent(builder, includeDefaults); + mapperBuilder.similarity.toXContent(builder, includeDefaults); + mapperBuilder.eagerGlobalOrdinals.toXContent(builder, includeDefaults); + mapperBuilder.positionIncrementGap.toXContent(builder, includeDefaults); + mapperBuilder.fieldData.toXContent(builder, includeDefaults); + mapperBuilder.freqFilter.toXContent(builder, includeDefaults); + mapperBuilder.indexPrefixes.toXContent(builder, includeDefaults); + mapperBuilder.indexPhrases.toXContent(builder, includeDefaults); } } diff --git a/server/src/test/java/org/opensearch/index/mapper/TextFieldMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/TextFieldMapperTests.java index be4379205e960..0bd47f9e7e4c1 100644 --- a/server/src/test/java/org/opensearch/index/mapper/TextFieldMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/TextFieldMapperTests.java @@ -270,10 +270,44 @@ public void testBWCSerialization() throws IOException { b.startObject("subfield").field("type", "long").endObject(); } b.endObject(); + b.field("store", true); + b.field("similarity", "BM25"); + b.field("index_options", "offsets"); + b.field("norms", false); + b.field("term_vector", "yes"); + b.field("position_increment_gap", 0); + b.startObject("fielddata_frequency_filter"); + { + b.field("min", 0.001); + b.field("max", 0.1); + b.field("min_segment_size", 500); + } + b.endObject(); + b.field("eager_global_ordinals", true); + b.field("index_phrases", true); + b.startObject("index_prefixes"); + { + b.field("min_chars", 1); + b.field("max_chars", 10); + } + b.endObject(); + b.startObject("meta"); + { + b.field("unit", "min"); + } + b.endObject(); + b.startArray("copy_to"); + { + b.value("target"); + } + b.endArray(); })); - assertEquals( - "{\"_doc\":{\"properties\":{\"field\":{\"type\":\"text\",\"fields\":{\"subfield\":{\"type\":\"long\"}},\"fielddata\":true}}}}", + "{\"_doc\":{\"properties\":{\"field\":{\"type\":\"text\",\"store\":true,\"fields\":{\"subfield\":{\"type\":\"long\"}}," + + "\"copy_to\":[\"target\"],\"meta\":{\"unit\":\"min\"},\"index_options\":\"offsets\",\"term_vector\":\"yes\",\"norms\":false," + + "\"similarity\":\"BM25\",\"eager_global_ordinals\":true,\"position_increment_gap\":0," + + "\"fielddata\":true,\"fielddata_frequency_filter\":{\"min\":0.001,\"max\":0.1,\"min_segment_size\":500}," + + "\"index_prefixes\":{\"min_chars\":1,\"max_chars\":10},\"index_phrases\":true}}}}", Strings.toString(XContentType.JSON, mapperService.documentMapper()) ); }