Skip to content

Commit

Permalink
Add specific test for serializing all mapping parameter values (#61844)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
romseygeek authored Sep 2, 2020
1 parent 76fa500 commit 3269d1b
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
}
});
private final Parameter<Double> nullValue = new Parameter<>("null_value", false, () -> null,
(n, c, o) -> XContentMapValues.nodeDoubleValue(o), m -> toType(m).nullValue);
(n, c, o) -> o == null ? null : XContentMapValues.nodeDoubleValue(o), m -> toType(m).nullValue).acceptsNull();

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
private final Parameter<Boolean> hasNorms
= Parameter.boolParam("norms", false, m -> toType(m).fieldType.omitNorms() == false, false);
private final Parameter<SimilarityProvider> similarity = new Parameter<>("similarity", false, () -> null,
(n, c, o) -> TypeParsers.resolveSimilarity(c, n, o.toString()), m -> toType(m).similarity);
(n, c, o) -> TypeParsers.resolveSimilarity(c, n, o), m -> toType(m).similarity)
.setSerializer((b, f, v) -> b.field(f, v == null ? null : v.name()), v -> v == null ? null : v.name())
.acceptsNull();

private final Parameter<String> normalizer
= Parameter.stringParam("normalizer", false, m -> toType(m).normalizerName, "default");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,11 @@ public static List<String> parseCopyFields(Object propNode) {
return copyFields;
}

public static SimilarityProvider resolveSimilarity(Mapper.TypeParser.ParserContext parserContext, String name, String value) {
SimilarityProvider similarityProvider = parserContext.getSimilarity(value);
public static SimilarityProvider resolveSimilarity(Mapper.TypeParser.ParserContext parserContext, String name, Object value) {
if (value == null) {
return null; // use default
}
SimilarityProvider similarityProvider = parserContext.getSimilarity(value.toString());
if (similarityProvider == null) {
throw new MapperParsingException("Unknown Similarity type [" + value + "] for field [" + name + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,18 @@ public void testEnableNorms() throws IOException {
assertEquals(0, fieldNamesFields.length);
}

public void testConfigureSimilarity() throws IOException {
MapperService mapperService = createMapperService(
fieldMapping(b -> b.field("type", "keyword").field("similarity", "boolean"))
);
MappedFieldType ft = mapperService.documentMapper().fieldTypes().get("field");
assertEquals("boolean", ft.getTextSearchInfo().getSimilarity().name());

IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> merge(mapperService, fieldMapping(b -> b.field("type", "keyword").field("similarity", "BM25"))));
assertThat(e.getMessage(), containsString("Cannot update parameter [similarity] from [boolean] to [BM25]"));
}

public void testNormalizer() throws IOException {
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "keyword").field("normalizer", "lowercase")));
ParsedDocument doc = mapper.parse(source(b -> b.field("field", "AbC")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,4 +228,9 @@ private XContentBuilder mappingsToJson(ToXContent builder, boolean includeDefaul
ToXContent.Params params = includeDefaults ? new ToXContent.MapParams(Map.of("include_defaults", "true")) : ToXContent.EMPTY_PARAMS;
return mapping(b -> builder.toXContent(b, params));
}

@Override
public void testMinimalToMaximal() {
assumeFalse("`include_defaults` includes unsupported properties in non-parametrized mappers", false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
Expand Down Expand Up @@ -63,6 +64,8 @@ public abstract class MapperServiceTestCase extends ESTestCase {

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

protected static final ToXContent.Params INCLUDE_DEFAULTS = new ToXContent.MapParams(Map.of("include_defaults", "true"));

protected Collection<? extends Plugin> getPlugins() {
return emptyList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@ public final void testMinimalSerializesToItself() throws IOException {
assertParseMinimalWarnings();
}

// TODO make this final once we remove FieldMapperTestCase2
public void testMinimalToMaximal() throws IOException {
XContentBuilder orig = JsonXContent.contentBuilder().startObject();
createMapperService(fieldMapping(this::minimalMapping)).documentMapper().mapping().toXContent(orig, INCLUDE_DEFAULTS);
orig.endObject();
XContentBuilder parsedFromOrig = JsonXContent.contentBuilder().startObject();
createMapperService(orig).documentMapper().mapping().toXContent(parsedFromOrig, INCLUDE_DEFAULTS);
parsedFromOrig.endObject();
assertEquals(Strings.toString(orig), Strings.toString(parsedFromOrig));
assertParseMinimalWarnings();
}

protected void assertParseMinimalWarnings() {
// Most mappers don't emit any warnings
}
Expand Down

0 comments on commit 3269d1b

Please sign in to comment.