Skip to content

Commit af01cce

Browse files
authored
Add specific test for serializing all mapping parameter values (#61844) (#61877)
This commit adds a test to MapperTestCase that explicitly checks that a mapper can serialize all its default values, and that this serialization can then be re-parsed. Note that the test is disabled for non-parametrized mappers as their serialization may in some cases output parameters that are not accepted. Gradually moving all mappers to parametrized form will address this. The commit also contains a fix to keyword mappers, which were not correctly serializing the similarity parameter; this partially addresses #61563. It also enables `null` as a value for `null_value` on `scaled_float`, as a follow-up to #61798
1 parent 2a02c6e commit af01cce

File tree

7 files changed

+43
-4
lines changed

7 files changed

+43
-4
lines changed

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
9393
}
9494
});
9595
private final Parameter<Double> nullValue = new Parameter<>("null_value", false, () -> null,
96-
(n, c, o) -> XContentMapValues.nodeDoubleValue(o), m -> toType(m).nullValue);
96+
(n, c, o) -> o == null ? null : XContentMapValues.nodeDoubleValue(o), m -> toType(m).nullValue).acceptsNull();
9797

9898
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
9999

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
100100
private final Parameter<Boolean> hasNorms
101101
= Parameter.boolParam("norms", false, m -> toType(m).fieldType.omitNorms() == false, false);
102102
private final Parameter<SimilarityProvider> similarity = new Parameter<>("similarity", false, () -> null,
103-
(n, c, o) -> TypeParsers.resolveSimilarity(c, n, o.toString()), m -> toType(m).similarity);
103+
(n, c, o) -> TypeParsers.resolveSimilarity(c, n, o), m -> toType(m).similarity)
104+
.setSerializer((b, f, v) -> b.field(f, v == null ? null : v.name()), v -> v == null ? null : v.name())
105+
.acceptsNull();
104106

105107
private final Parameter<String> normalizer
106108
= Parameter.stringParam("normalizer", false, m -> toType(m).normalizerName, "default");

server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,11 @@ public static List<String> parseCopyFields(Object propNode) {
390390
return copyFields;
391391
}
392392

393-
public static SimilarityProvider resolveSimilarity(Mapper.TypeParser.ParserContext parserContext, String name, String value) {
394-
SimilarityProvider similarityProvider = parserContext.getSimilarity(value);
393+
public static SimilarityProvider resolveSimilarity(Mapper.TypeParser.ParserContext parserContext, String name, Object value) {
394+
if (value == null) {
395+
return null; // use default
396+
}
397+
SimilarityProvider similarityProvider = parserContext.getSimilarity(value.toString());
395398
if (similarityProvider == null) {
396399
throw new MapperParsingException("Unknown Similarity type [" + value + "] for field [" + name + "]");
397400
}

server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,18 @@ public void testEnableNorms() throws IOException {
250250
assertEquals(0, fieldNamesFields.length);
251251
}
252252

253+
public void testConfigureSimilarity() throws IOException {
254+
MapperService mapperService = createMapperService(
255+
fieldMapping(b -> b.field("type", "keyword").field("similarity", "boolean"))
256+
);
257+
MappedFieldType ft = mapperService.documentMapper().fieldTypes().get("field");
258+
assertEquals("boolean", ft.getTextSearchInfo().getSimilarity().name());
259+
260+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
261+
() -> merge(mapperService, fieldMapping(b -> b.field("type", "keyword").field("similarity", "BM25"))));
262+
assertThat(e.getMessage(), containsString("Cannot update parameter [similarity] from [boolean] to [BM25]"));
263+
}
264+
253265
public void testNormalizer() throws IOException {
254266
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "keyword").field("normalizer", "lowercase")));
255267
ParsedDocument doc = mapper.parse(source(b -> b.field("field", "AbC")));

test/framework/src/main/java/org/elasticsearch/index/mapper/FieldMapperTestCase2.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,9 @@ private XContentBuilder mappingsToJson(ToXContent builder, boolean includeDefaul
230230
: ToXContent.EMPTY_PARAMS;
231231
return mapping(b -> builder.toXContent(b, params));
232232
}
233+
234+
@Override
235+
public void testMinimalToMaximal() {
236+
assumeFalse("`include_defaults` includes unsupported properties in non-parametrized mappers", false);
237+
}
233238
}

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.elasticsearch.common.bytes.BytesReference;
3131
import org.elasticsearch.common.compress.CompressedXContent;
3232
import org.elasticsearch.common.settings.Settings;
33+
import org.elasticsearch.common.xcontent.ToXContent;
3334
import org.elasticsearch.common.xcontent.XContentBuilder;
3435
import org.elasticsearch.common.xcontent.XContentFactory;
3536
import org.elasticsearch.common.xcontent.XContentType;
@@ -49,6 +50,7 @@
4950

5051
import java.io.IOException;
5152
import java.util.Collection;
53+
import java.util.Collections;
5254

5355
import static java.util.Collections.emptyList;
5456
import static java.util.Collections.emptyMap;
@@ -63,6 +65,9 @@ public abstract class MapperServiceTestCase extends ESTestCase {
6365

6466
protected static final Settings SETTINGS = Settings.builder().put("index.version.created", Version.CURRENT).build();
6567

68+
protected static final ToXContent.Params INCLUDE_DEFAULTS
69+
= new ToXContent.MapParams(Collections.singletonMap("include_defaults", "true"));
70+
6671
protected Collection<? extends Plugin> getPlugins() {
6772
return emptyList();
6873
}

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@ public final void testMinimalSerializesToItself() throws IOException {
6363
assertParseMinimalWarnings();
6464
}
6565

66+
// TODO make this final once we remove FieldMapperTestCase2
67+
public void testMinimalToMaximal() throws IOException {
68+
XContentBuilder orig = JsonXContent.contentBuilder().startObject();
69+
createMapperService(fieldMapping(this::minimalMapping)).documentMapper().mapping().toXContent(orig, INCLUDE_DEFAULTS);
70+
orig.endObject();
71+
XContentBuilder parsedFromOrig = JsonXContent.contentBuilder().startObject();
72+
createMapperService(orig).documentMapper().mapping().toXContent(parsedFromOrig, INCLUDE_DEFAULTS);
73+
parsedFromOrig.endObject();
74+
assertEquals(Strings.toString(orig), Strings.toString(parsedFromOrig));
75+
assertParseMinimalWarnings();
76+
}
77+
6678
protected void assertParseMinimalWarnings() {
6779
// Most mappers don't emit any warnings
6880
}

0 commit comments

Comments
 (0)