Skip to content

Commit

Permalink
Replaced _data_stream_timestamp meta field's 'path' option with 'enab…
Browse files Browse the repository at this point in the history 
…led' option (#59728)

Backport #59727 to 7.x

and adjusted exception messages.

Relates to #59076
  • Loading branch information
@martijnvg
martijnvg committed Jul 16, 2020
1 parent 2c97de5 commit 268f4f5
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 119 deletions.
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,10 @@ public String getTimestampField() {
/**
* @return a mapping snippet for a backing index with `_data_stream_timestamp` meta field mapper properly configured.
*/
public Map<String, Object> getDataSteamMappingSnippet() {
public Map<String, Object> getDataStreamMappingSnippet() {
// _data_stream_timestamp meta fields default to @timestamp:
return singletonMap(MapperService.SINGLE_MAPPING_NAME, singletonMap("_data_stream_timestamp",
singletonMap("path", FIXED_TIMESTAMP_FIELD)));
singletonMap("enabled", true)));
}

@Override
Expand Down
9 changes: 5 additions & 4 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateDataStreamService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,11 @@ public static void validateTimestampFieldMapping(String timestampFieldName, Mapp

Map<String, Object> parsedTemplateMapping =
MapperService.parseMapping(NamedXContentRegistry.EMPTY, mapperService.documentMapper().mappingSource().string());
String configuredPath = ObjectPath.eval("_doc._data_stream_timestamp.path", parsedTemplateMapping);
if (timestampFieldName.equals(configuredPath) == false) {
throw new IllegalArgumentException("[_data_stream_timestamp] meta field doesn't point to data stream timestamp field [" +
timestampFieldName + "]");
Boolean enabled = ObjectPath.eval("_doc._data_stream_timestamp.enabled", parsedTemplateMapping);
// Sanity check: if this fails then somehow the mapping for _data_stream_timestamp has been overwritten and
// that would be a bug.
if (enabled == null || enabled == false) {
throw new IllegalStateException("[_data_stream_timestamp] meta field has been disabled");
}

// Sanity check (this validation logic should already have been executed when merging mappings):
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ public static List<CompressedXContent> collectMappings(final ClusterState state,
// Only if template has data stream definition this should be added and
// adding this template last, since _timestamp field should have highest precedence:
Optional.ofNullable(template.getDataStreamTemplate())
.map(ComposableIndexTemplate.DataStreamTemplate::getDataSteamMappingSnippet)
.map(ComposableIndexTemplate.DataStreamTemplate::getDataStreamMappingSnippet)
.map(mapping -> {
try (XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {
builder.value(mapping);
Expand Down
10 changes: 5 additions & 5 deletions ...r/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1556,11 +1556,11 @@ public Map<String, MetadataFieldMapper.TypeParser> getMetadataMappers() {
public MetadataFieldMapper.Builder<?> parse(String name,
Map<String, Object> node,
ParserContext parserContext) throws MapperParsingException {
String path = (String) node.remove("path");
Boolean enabled = (Boolean) node.remove("enabled");
return new MetadataFieldMapper.Builder(name, new FieldType()) {
@Override
public MetadataFieldMapper build(Mapper.BuilderContext context) {
return newInstance(path);
return newInstance(enabled);
}
};
}
Expand All @@ -1570,7 +1570,7 @@ public MetadataFieldMapper getDefault(MappedFieldType fieldType, ParserContext p
return newInstance(null);
}

MetadataFieldMapper newInstance(String path) {
MetadataFieldMapper newInstance(Boolean enabled) {
FieldType fieldType = new FieldType();
fieldType.freeze();
MappedFieldType mappedFieldType =
Expand Down Expand Up @@ -1603,12 +1603,12 @@ protected void parseCreateField(ParseContext context) throws IOException {

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
if (path == null) {
if (enabled == null) {
return builder;
}

builder.startObject(simpleName());
builder.field("path", path);
builder.field("enabled", enabled);
return builder.endObject();
}

Expand Down
2 changes: 1 addition & 1 deletion test/framework/src/main/java/org/elasticsearch/cluster/DataStreamTestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public static String generateMapping(String timestampFieldName) {
public static String generateMapping(String timestampFieldName, String type) {
return "{\n" +
" \"_data_stream_timestamp\": {\n" +
" \"path\": \"" + timestampFieldName + "\"\n" +
" \"enabled\": true\n" +
" }," +
" \"properties\": {\n" +
" \"" + timestampFieldName + "\": {\n" +
Expand Down
6 changes: 3 additions & 3 deletions ...data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ public void testTimeStampValidationInvalidFieldMapping() throws Exception {
);
assertThat(
e.getCause().getCause().getMessage(),
equalTo("the configured timestamp field [@timestamp] is of type [keyword], but [date,date_nanos] is expected")
equalTo("data stream timestamp field [@timestamp] is of type [keyword], but [date,date_nanos] is expected")
);
}

Expand Down Expand Up @@ -632,7 +632,7 @@ public void testUpdateMappingViaDataStream() throws Exception {
"properties",
Map.of("@timestamp", Map.of("type", "date")),
"_data_stream_timestamp",
Map.of("path", "@timestamp")
Map.of("enabled", true)
);
GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("logs-foobar").get();
assertThat(getMappingsResponse.getMappings().size(), equalTo(2));
Expand All @@ -643,7 +643,7 @@ public void testUpdateMappingViaDataStream() throws Exception {
"properties",
Map.of("@timestamp", Map.of("type", "date"), "my_field", Map.of("type", "keyword")),
"_data_stream_timestamp",
Map.of("path", "@timestamp")
Map.of("enabled", true)
);
client().admin()
.indices()
Expand Down
52 changes: 26 additions & 26 deletions .../main/java/org/elasticsearch/xpack/datastreams/mapper/DataStreamTimestampFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.mapper.DateFieldMapper;
import org.elasticsearch.index.mapper.DocumentFieldMappers;
import org.elasticsearch.index.mapper.FieldMapper;
Expand All @@ -40,6 +41,7 @@
public class DataStreamTimestampFieldMapper extends MetadataFieldMapper {

public static final String NAME = "_data_stream_timestamp";
private static final String DEFAULT_PATH = "@timestamp";

public static class Defaults {

Expand Down Expand Up @@ -78,19 +80,19 @@ public Query existsQuery(QueryShardContext context) {

public static class Builder extends MetadataFieldMapper.Builder<Builder> {

private String path;
private boolean enabled;

public Builder() {
super(NAME, Defaults.TIMESTAMP_FIELD_TYPE);
}

public void setPath(String path) {
this.path = path;
public void setEnabled(boolean enabled) {
this.enabled = enabled;
}

@Override
public MetadataFieldMapper build(BuilderContext context) {
return new DataStreamTimestampFieldMapper(fieldType, new TimestampFieldType(), path);
return new DataStreamTimestampFieldMapper(fieldType, new TimestampFieldType(), enabled);
}
}

Expand All @@ -104,8 +106,8 @@ public MetadataFieldMapper.Builder<?> parse(String name, Map<String, Object> nod
Map.Entry<String, Object> entry = iterator.next();
String fieldName = entry.getKey();
Object fieldNode = entry.getValue();
if (fieldName.equals("path")) {
builder.setPath((String) fieldNode);
if (fieldName.equals("enabled")) {
builder.setEnabled(XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled"));
iterator.remove();
}
}
Expand All @@ -114,32 +116,34 @@ public MetadataFieldMapper.Builder<?> parse(String name, Map<String, Object> nod

@Override
public MetadataFieldMapper getDefault(MappedFieldType fieldType, ParserContext parserContext) {
return new DataStreamTimestampFieldMapper(Defaults.TIMESTAMP_FIELD_TYPE, new TimestampFieldType(), null);
return new DataStreamTimestampFieldMapper(Defaults.TIMESTAMP_FIELD_TYPE, new TimestampFieldType(), false);
}
}

private final String path;
private final boolean enabled;

private DataStreamTimestampFieldMapper(FieldType fieldType, MappedFieldType mappedFieldType, String path) {
private DataStreamTimestampFieldMapper(FieldType fieldType, MappedFieldType mappedFieldType, boolean enabled) {
super(fieldType, mappedFieldType);
this.path = path;
this.path = DEFAULT_PATH;
this.enabled = enabled;
}

public void validate(DocumentFieldMappers lookup) {
if (path == null) {
if (enabled == false) {
// not configured, so skip the validation
return;
}

Mapper mapper = lookup.getMapper(path);
if (mapper == null) {
throw new IllegalArgumentException("the configured timestamp field [" + path + "] does not exist");
throw new IllegalArgumentException("data stream timestamp field [" + path + "] does not exist");
}

if (DateFieldMapper.CONTENT_TYPE.equals(mapper.typeName()) == false
&& DateFieldMapper.DATE_NANOS_CONTENT_TYPE.equals(mapper.typeName()) == false) {
throw new IllegalArgumentException(
"the configured timestamp field ["
"data stream timestamp field ["
+ path
+ "] is of type ["
+ mapper.typeName()
Expand All @@ -153,19 +157,19 @@ public void validate(DocumentFieldMappers lookup) {

DateFieldMapper dateFieldMapper = (DateFieldMapper) mapper;
if (dateFieldMapper.fieldType().isSearchable() == false) {
throw new IllegalArgumentException("the configured timestamp field [" + path + "] is not indexed");
throw new IllegalArgumentException("data stream timestamp field [" + path + "] is not indexed");
}
if (dateFieldMapper.fieldType().hasDocValues() == false) {
throw new IllegalArgumentException("the configured timestamp field [" + path + "] doesn't have doc values");
throw new IllegalArgumentException("data stream timestamp field [" + path + "] doesn't have doc values");
}
if (dateFieldMapper.getNullValue() != null) {
throw new IllegalArgumentException(
"the configured timestamp field [" + path + "] has disallowed [null_value] attribute specified"
"data stream timestamp field [" + path + "] has disallowed [null_value] attribute specified"
);
}
if (dateFieldMapper.getIgnoreMalformed().explicit()) {
throw new IllegalArgumentException(
"the configured timestamp field [" + path + "] has disallowed [ignore_malformed] attribute specified"
"data stream timestamp field [" + path + "] has disallowed [ignore_malformed] attribute specified"
);
}

Expand All @@ -185,18 +189,14 @@ public void validate(DocumentFieldMappers lookup) {
// All other configured attributes are not allowed:
if (configuredSettings.isEmpty() == false) {
throw new IllegalArgumentException(
"the configured timestamp field [@timestamp] has disallowed attributes: " + configuredSettings.keySet()
"data stream timestamp field [@timestamp] has disallowed attributes: " + configuredSettings.keySet()
);
}
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

public String getPath() {
return path;
}

@Override
public void preParse(ParseContext context) throws IOException {}

Expand All @@ -208,7 +208,7 @@ protected void parseCreateField(ParseContext context) throws IOException {

@Override
public void postParse(ParseContext context) throws IOException {
if (path == null) {
if (enabled == false) {
// not configured, so skip the validation
return;
}
Expand All @@ -228,12 +228,12 @@ public void postParse(ParseContext context) throws IOException {

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
if (path == null) {
if (enabled == false) {
return builder;
}

builder.startObject(simpleName());
builder.field("path", path);
builder.field("enabled", enabled);
return builder.endObject();
}

Expand All @@ -255,8 +255,8 @@ protected boolean docValuesByDefault() {
@Override
protected void mergeOptions(FieldMapper other, List<String> conflicts) {
DataStreamTimestampFieldMapper otherTimestampFieldMapper = (DataStreamTimestampFieldMapper) other;
if (Objects.equals(path, otherTimestampFieldMapper.path) == false) {
conflicts.add("cannot update path setting for [_data_stream_timestamp]");
if (Objects.equals(enabled, otherTimestampFieldMapper.enabled) == false) {
conflicts.add("cannot update enabled setting for [_data_stream_timestamp]");
}
}
}

0 comments on commit 268f4f5

Please sign in to comment.