Skip to content

Add null-field checks to shape field mappers (#71999) #72025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,9 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
assumeFalse("Test implemented in a follow up", true);
return null;
}

@Override
protected boolean allowsNullValues() {
return false; // TODO should this allow null values?
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ protected void checkIncomingMergeType(FieldMapper mergeWith) {

@Override
protected void index(ParseContext context, Geometry geometry) throws IOException {
if (geometry == null) {
return;
}
context.doc().addAll(indexer.indexShape(geometry));
createFieldNamesField(context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,9 @@ String strategy() {

@Override
protected void index(ParseContext context, ShapeBuilder<?, ?, ?> shapeBuilder) throws IOException {
if (shapeBuilder == null) {
return;
}
Shape shape = shapeBuilder.buildS4J();
if (fieldType().pointsOnly()) {
// index configured for pointsOnly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,19 @@ private BiFunction<MappedFieldType, Supplier<SearchLookup>, IndexFieldData<?>> f
.fielddataBuilder("test", lookupSource)
.build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService());
}

public final void testNullInput() throws Exception {
DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
if (allowsNullValues()) {
ParsedDocument doc = mapper.parse(source(b -> b.nullField("field")));
assertThat(doc.docs().get(0).getFields("field").length, equalTo(0));
assertThat(doc.docs().get(0).getFields("_field_names").length, equalTo(0));
} else {
expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.nullField("field"))));
}
}

protected boolean allowsNullValues() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,9 @@ protected String generateRandomInputValue(MappedFieldType ft) {
protected void randomFetchTestFieldConfig(XContentBuilder b) throws IOException {
b.field("type", "constant_keyword").field("value", randomAlphaOfLengthBetween(1, 10));
}

@Override
protected boolean allowsNullValues() {
return false; // null is an error for constant keyword
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ public GeoShapeWithDocValuesFieldMapper(String simpleName, MappedFieldType mappe

@Override
protected void index(ParseContext context, Geometry geometry) throws IOException {
if (geometry == null) {
return;
}
List<IndexableField> fields = indexer.indexShape(geometry);
if (fieldType().isSearchable()) {
context.doc().addAll(fields);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ public ShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,

@Override
protected void index(ParseContext context, Geometry geometry) throws IOException {
if (geometry == null) {
return;
}
context.doc().addAll(indexer.indexShape(geometry));
createFieldNamesField(context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,9 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
assumeFalse("Test implemented in a follow up", true);
return null;
}

@Override
protected boolean allowsNullValues() {
return false; // TODO should this allow null values?
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,9 @@ protected Object generateRandomInputValue(MappedFieldType ft) {
assumeFalse("doesn't support docvalues_fetcher", true);
return null;
}

@Override
protected boolean allowsNullValues() {
return false;
}
}