Skip to content

Commit 03c3d6a

Browse files
authored
Allow [null] values in [null_value] (#61798)
Several field mappers have a null_value parameter, that allows you to specify a placeholder value to insert into a document if the incoming value for that field is null. The default value for this is always null, meaning "add no placeholder". However, we explicitly bar users from setting this parameter directly to null (done in #7978, in order to fix an NPE). This exclusion means that if a mapper is serialized with include_defaults, then we either need to special-case null_value to ensure that it is not output when it holds the default value, or we find that the resulting serialized form cannot be used to create a mapping. This stops us doing some useful generic testing of mappers. This commit permits null as a parameter value for null_value, and changes the tests to check that it is a) permissible and b) applied without throwing errors. As part of the testing changes, a new base class MapperServiceTestCase is refactored from MapperTestCase, holding the various helper methods related to building mappings but not the single-mapper specific abstract methods. Closes #58823
1 parent ba87bad commit 03c3d6a

File tree

9 files changed

+214
-194
lines changed

9 files changed

+214
-194
lines changed

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

+1-4
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ public T parse(String name, Map<String, Object> node, Map<String, Object> params
9696
Object propNode = entry.getValue();
9797

9898
if (Names.NULL_VALUE.match(propName, LoggingDeprecationHandler.INSTANCE)) {
99-
if (propNode == null) {
100-
throw new MapperParsingException("Property [null_value] cannot be null.");
101-
}
10299
nullValue = propNode;
103100
iterator.remove();
104101
}
@@ -146,7 +143,7 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {
146143
@Override
147144
public void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
148145
super.doXContentBody(builder, includeDefaults, params);
149-
if (nullValue != null) {
146+
if (nullValue != null || includeDefaults) {
150147
builder.field(Names.NULL_VALUE.getPreferredName(), nullValue);
151148
}
152149
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
8282
private final Parameter<Boolean> stored = Parameter.storeParam(m -> toType(m).stored, false);
8383

8484
private final Parameter<Boolean> nullValue = new Parameter<>("null_value", false, () -> null,
85-
(n, c, o) -> XContentMapValues.nodeBooleanValue(o), m -> toType(m).nullValue);
85+
(n, c, o) -> o == null ? null : XContentMapValues.nodeBooleanValue(o), m -> toType(m).nullValue)
86+
.acceptsNull();
8687

8788
private final Parameter<Float> boost = Parameter.boostParam();
8889
private final Parameter<Map<String, String>> meta = Parameter.metaParam();

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
195195
(n, c, o) -> LocaleUtils.parse(o.toString()), m -> toType(m).locale);
196196

197197
private final Parameter<String> nullValue
198-
= Parameter.stringParam("null_value", false, m -> toType(m).nullValueAsString, null);
198+
= Parameter.stringParam("null_value", false, m -> toType(m).nullValueAsString, null).acceptsNull();
199199
private final Parameter<Boolean> ignoreMalformed;
200200

201201
private final Resolution resolution;

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,15 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
6868

6969
private final Parameter<Boolean> ignoreMalformed;
7070
private final Parameter<InetAddress> nullValue = new Parameter<>("null_value", false, () -> null,
71-
(n, c, o) -> InetAddresses.forString(o.toString()), m -> toType(m).nullValue)
71+
(n, c, o) -> o == null ? null : InetAddresses.forString(o.toString()), m -> toType(m).nullValue)
7272
.setSerializer((b, f, v) -> {
7373
if (v == null) {
7474
b.nullField(f);
7575
} else {
7676
b.field(f, InetAddresses.toAddrString(v));
7777
}
78-
}, NetworkAddress::format);
78+
}, NetworkAddress::format)
79+
.acceptsNull();
7980

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

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
8686
private final Parameter<Boolean> hasDocValues = Parameter.docValuesParam(m -> toType(m).hasDocValues, true);
8787
private final Parameter<Boolean> stored = Parameter.storeParam(m -> toType(m).fieldType.stored(), false);
8888

89-
private final Parameter<String> nullValue = Parameter.stringParam("null_value", false, m -> toType(m).nullValue, null);
89+
private final Parameter<String> nullValue
90+
= Parameter.stringParam("null_value", false, m -> toType(m).nullValue, null).acceptsNull();
9091

9192
private final Parameter<Boolean> eagerGlobalOrdinals
9293
= Parameter.boolParam("eager_global_ordinals", true, m -> toType(m).eagerGlobalOrdinals, false);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public Builder(String name, NumberType type, boolean ignoreMalformedByDefault, b
109109
this.coerce
110110
= Parameter.explicitBoolParam("coerce", true, m -> toType(m).coerce, coerceByDefault);
111111
this.nullValue = new Parameter<>("null_value", false, () -> null,
112-
(n, c, o) -> type.parse(o, false), m -> toType(m).nullValue);
112+
(n, c, o) -> o == null ? null : type.parse(o, false), m -> toType(m).nullValue).acceptsNull();
113113
}
114114

115115
Builder nullValue(Number number) {
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,3 @@
1-
package org.elasticsearch.index.mapper;
2-
3-
import org.elasticsearch.common.Strings;
4-
import org.elasticsearch.common.compress.CompressedXContent;
5-
import org.elasticsearch.common.settings.Settings;
6-
import org.elasticsearch.common.xcontent.XContentFactory;
7-
import org.elasticsearch.index.IndexService;
8-
import org.elasticsearch.test.ESSingleNodeTestCase;
9-
10-
import static org.hamcrest.Matchers.containsString;
11-
import static org.hamcrest.Matchers.either;
12-
import static org.hamcrest.Matchers.equalTo;
13-
141
/*
152
* Licensed to Elasticsearch under one or more contributor
163
* license agreements. See the NOTICE file distributed with
@@ -30,32 +17,33 @@
3017
* under the License.
3118
*/
3219

33-
public class NullValueTests extends ESSingleNodeTestCase {
20+
package org.elasticsearch.index.mapper;
21+
22+
import org.elasticsearch.common.Strings;
23+
import org.elasticsearch.common.xcontent.ToXContent;
24+
import org.elasticsearch.common.xcontent.XContentBuilder;
25+
import org.elasticsearch.common.xcontent.json.JsonXContent;
26+
27+
import java.util.Map;
28+
29+
import static org.hamcrest.Matchers.containsString;
30+
31+
public class NullValueTests extends MapperServiceTestCase {
32+
3433
public void testNullNullValue() throws Exception {
35-
IndexService indexService = createIndex("test", Settings.builder().build());
34+
3635
String[] typesToTest = {"integer", "long", "double", "float", "short", "date", "ip", "keyword", "boolean", "byte", "geo_point"};
3736

3837
for (String type : typesToTest) {
39-
String mapping = Strings.toString(XContentFactory.jsonBuilder()
40-
.startObject()
41-
.startObject("type")
42-
.startObject("properties")
43-
.startObject("numeric")
44-
.field("type", type)
45-
.field("null_value", (String) null)
46-
.endObject()
47-
.endObject()
48-
.endObject()
49-
.endObject());
50-
51-
try {
52-
indexService.mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping));
53-
fail("Test should have failed because [null_value] was null.");
54-
} catch (MapperParsingException e) {
55-
assertThat(e.getMessage(),
56-
either(equalTo("Property [null_value] cannot be null."))
57-
.or(containsString("must not have a [null] value")));
58-
}
38+
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", type).nullField("null_value")));
39+
40+
mapper.parse(source(b -> b.nullField("field")));
41+
42+
ToXContent.Params params = new ToXContent.MapParams(Map.of("include_defaults", "true"));
43+
XContentBuilder b = JsonXContent.contentBuilder().startObject();
44+
mapper.mapping().toXContent(b, params);
45+
b.endObject();
46+
assertThat(Strings.toString(b), containsString("\"null_value\":null"));
5947
}
6048
}
6149
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.index.mapper;
21+
22+
import org.apache.lucene.analysis.standard.StandardAnalyzer;
23+
import org.apache.lucene.index.IndexReader;
24+
import org.apache.lucene.index.IndexWriterConfig;
25+
import org.apache.lucene.index.RandomIndexWriter;
26+
import org.apache.lucene.store.Directory;
27+
import org.elasticsearch.Version;
28+
import org.elasticsearch.cluster.metadata.IndexMetadata;
29+
import org.elasticsearch.common.CheckedConsumer;
30+
import org.elasticsearch.common.bytes.BytesReference;
31+
import org.elasticsearch.common.compress.CompressedXContent;
32+
import org.elasticsearch.common.settings.Settings;
33+
import org.elasticsearch.common.xcontent.XContentBuilder;
34+
import org.elasticsearch.common.xcontent.XContentFactory;
35+
import org.elasticsearch.common.xcontent.XContentType;
36+
import org.elasticsearch.common.xcontent.json.JsonXContent;
37+
import org.elasticsearch.index.IndexSettings;
38+
import org.elasticsearch.index.analysis.AnalyzerScope;
39+
import org.elasticsearch.index.analysis.IndexAnalyzers;
40+
import org.elasticsearch.index.analysis.NamedAnalyzer;
41+
import org.elasticsearch.index.query.QueryShardContext;
42+
import org.elasticsearch.index.similarity.SimilarityService;
43+
import org.elasticsearch.indices.IndicesModule;
44+
import org.elasticsearch.indices.mapper.MapperRegistry;
45+
import org.elasticsearch.plugins.MapperPlugin;
46+
import org.elasticsearch.plugins.Plugin;
47+
import org.elasticsearch.script.ScriptService;
48+
import org.elasticsearch.test.ESTestCase;
49+
50+
import java.io.IOException;
51+
import java.util.Collection;
52+
import java.util.Map;
53+
54+
import static java.util.Collections.emptyList;
55+
import static java.util.Collections.emptyMap;
56+
import static java.util.stream.Collectors.toList;
57+
import static org.mockito.Matchers.anyObject;
58+
import static org.mockito.Matchers.anyString;
59+
import static org.mockito.Mockito.mock;
60+
import static org.mockito.Mockito.when;
61+
62+
public abstract class MapperServiceTestCase extends ESTestCase {
63+
64+
protected static final Settings SETTINGS = Settings.builder().put("index.version.created", Version.CURRENT).build();
65+
66+
protected Collection<? extends Plugin> getPlugins() {
67+
return emptyList();
68+
}
69+
70+
protected Settings getIndexSettings() {
71+
return SETTINGS;
72+
}
73+
74+
protected IndexAnalyzers createIndexAnalyzers(IndexSettings indexSettings) {
75+
return new IndexAnalyzers(
76+
Map.of("default", new NamedAnalyzer("default", AnalyzerScope.INDEX, new StandardAnalyzer())),
77+
Map.of(),
78+
Map.of()
79+
);
80+
}
81+
82+
protected final String randomIndexOptions() {
83+
return randomFrom("docs", "freqs", "positions", "offsets");
84+
}
85+
86+
protected final DocumentMapper createDocumentMapper(XContentBuilder mappings) throws IOException {
87+
return createMapperService(mappings).documentMapper();
88+
}
89+
90+
protected final MapperService createMapperService(XContentBuilder mappings) throws IOException {
91+
return createMapperService(getIndexSettings(), mappings);
92+
}
93+
94+
/**
95+
* Create a {@link MapperService} like we would for an index.
96+
*/
97+
protected final MapperService createMapperService(Settings settings, XContentBuilder mapping) throws IOException {
98+
IndexMetadata meta = IndexMetadata.builder("index")
99+
.settings(Settings.builder().put("index.version.created", Version.CURRENT))
100+
.numberOfReplicas(0)
101+
.numberOfShards(1)
102+
.build();
103+
IndexSettings indexSettings = new IndexSettings(meta, settings);
104+
MapperRegistry mapperRegistry = new IndicesModule(
105+
getPlugins().stream().filter(p -> p instanceof MapperPlugin).map(p -> (MapperPlugin) p).collect(toList())
106+
).getMapperRegistry();
107+
ScriptService scriptService = new ScriptService(settings, emptyMap(), emptyMap());
108+
SimilarityService similarityService = new SimilarityService(indexSettings, scriptService, Map.of());
109+
MapperService mapperService = new MapperService(
110+
indexSettings,
111+
createIndexAnalyzers(indexSettings),
112+
xContentRegistry(),
113+
similarityService,
114+
mapperRegistry,
115+
() -> { throw new UnsupportedOperationException(); },
116+
() -> true
117+
);
118+
merge(mapperService, mapping);
119+
return mapperService;
120+
}
121+
122+
protected final void withLuceneIndex(
123+
MapperService mapperService,
124+
CheckedConsumer<RandomIndexWriter, IOException> builder,
125+
CheckedConsumer<IndexReader, IOException> test
126+
) throws IOException {
127+
try (
128+
Directory dir = newDirectory();
129+
RandomIndexWriter iw = new RandomIndexWriter(random(), dir, new IndexWriterConfig(mapperService.indexAnalyzer()))
130+
) {
131+
builder.accept(iw);
132+
try (IndexReader reader = iw.getReader()) {
133+
test.accept(reader);
134+
}
135+
}
136+
}
137+
138+
protected final SourceToParse source(CheckedConsumer<XContentBuilder, IOException> build) throws IOException {
139+
XContentBuilder builder = JsonXContent.contentBuilder().startObject();
140+
build.accept(builder);
141+
builder.endObject();
142+
return new SourceToParse("test", "1", BytesReference.bytes(builder), XContentType.JSON);
143+
}
144+
145+
/**
146+
* Merge a new mapping into the one in the provided {@link MapperService}.
147+
*/
148+
protected final void merge(MapperService mapperService, XContentBuilder mapping) throws IOException {
149+
mapperService.merge(null, new CompressedXContent(BytesReference.bytes(mapping)), MapperService.MergeReason.MAPPING_UPDATE);
150+
}
151+
152+
protected final XContentBuilder mapping(CheckedConsumer<XContentBuilder, IOException> buildFields) throws IOException {
153+
XContentBuilder builder = XContentFactory.jsonBuilder().startObject().startObject("_doc").startObject("properties");
154+
buildFields.accept(builder);
155+
return builder.endObject().endObject().endObject();
156+
}
157+
158+
protected final XContentBuilder fieldMapping(CheckedConsumer<XContentBuilder, IOException> buildField) throws IOException {
159+
return mapping(b -> {
160+
b.startObject("field");
161+
buildField.accept(b);
162+
b.endObject();
163+
});
164+
}
165+
166+
QueryShardContext createQueryShardContext(MapperService mapperService) {
167+
QueryShardContext queryShardContext = mock(QueryShardContext.class);
168+
when(queryShardContext.getMapperService()).thenReturn(mapperService);
169+
when(queryShardContext.fieldMapper(anyString())).thenAnswer(inv -> mapperService.fieldType(inv.getArguments()[0].toString()));
170+
when(queryShardContext.getIndexAnalyzers()).thenReturn(mapperService.getIndexAnalyzers());
171+
when(queryShardContext.getSearchQuoteAnalyzer(anyObject())).thenCallRealMethod();
172+
when(queryShardContext.getSearchAnalyzer(anyObject())).thenCallRealMethod();
173+
when(queryShardContext.getIndexSettings()).thenReturn(mapperService.getIndexSettings());
174+
when(queryShardContext.simpleMatchToIndexNames(anyObject())).thenAnswer(
175+
inv -> mapperService.simpleMatchToFullName(inv.getArguments()[0].toString())
176+
);
177+
return queryShardContext;
178+
}
179+
}

0 commit comments

Comments
 (0)