From 59ddbd5393117e076c43bdaf6d945e4a42616103 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 1 Nov 2019 16:03:11 +0000 Subject: [PATCH 1/8] Remove types from internal GetFieldMappings request/response objects --- .../get/GetFieldMappingsIndexRequest.java | 16 ++-- .../mapping/get/GetFieldMappingsRequest.java | 19 ++--- .../get/GetFieldMappingsRequestBuilder.java | 10 --- .../mapping/get/GetFieldMappingsResponse.java | 72 ++++++++++-------- .../get/TransportGetFieldMappingsAction.java | 4 +- .../TransportGetFieldMappingsIndexAction.java | 34 ++------- .../indices/RestGetFieldMappingAction.java | 17 ++--- .../get/GetFieldMappingsResponseTests.java | 31 ++++---- .../mapper/FieldFilterMapperPluginTests.java | 6 +- .../mapping/SimpleGetFieldMappingsIT.java | 75 +++++++------------ .../DocumentAndFieldLevelSecurityTests.java | 12 ++- .../integration/KibanaUserRoleIntegTests.java | 5 +- ...HistoryTemplateTransformMappingsTests.java | 3 - 13 files changed, 121 insertions(+), 183 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java index 576d3812c0cf4..b7dca8bde3455 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java @@ -19,10 +19,12 @@ package org.elasticsearch.action.admin.indices.mapping.get; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.OriginalIndices; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.single.shard.SingleShardRequest; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -33,13 +35,14 @@ public class GetFieldMappingsIndexRequest extends SingleShardRequest>> mappings; + private final Map> mappings; - GetFieldMappingsResponse(Map>> mappings) { + GetFieldMappingsResponse(Map> mappings) { this.mappings = mappings; } GetFieldMappingsResponse(StreamInput in) throws IOException { super(in); int size = in.readVInt(); - Map>> indexMapBuilder = new HashMap<>(size); + Map> indexMapBuilder = new HashMap<>(size); for (int i = 0; i < size; i++) { String index = in.readString(); - int typesSize = in.readVInt(); - Map> typeMapBuilder = new HashMap<>(typesSize); - for (int j = 0; j < typesSize; j++) { - String type = in.readString(); + Map fieldMapBuilder = new HashMap<>(); + if (in.getVersion().before(Version.V_8_0_0)) { + int typesSize = in.readVInt(); + assert typesSize <= 1; + for (int j = 0; j < typesSize; j++) { + in.readString(); // type + int fieldSize = in.readVInt(); + for (int k = 0; k < fieldSize; k++) { + fieldMapBuilder.put(in.readString(), new FieldMappingMetaData(in.readString(), in.readBytesReference())); + } + } + } else { int fieldSize = in.readVInt(); - Map fieldMapBuilder = new HashMap<>(fieldSize); for (int k = 0; k < fieldSize; k++) { fieldMapBuilder.put(in.readString(), new FieldMappingMetaData(in.readString(), in.readBytesReference())); } - typeMapBuilder.put(type, unmodifiableMap(fieldMapBuilder)); } - indexMapBuilder.put(index, unmodifiableMap(typeMapBuilder)); + indexMapBuilder.put(index, unmodifiableMap(fieldMapBuilder)); } mappings = unmodifiableMap(indexMapBuilder); } /** returns the retrieved field mapping. The return map keys are index, type, field (as specified in the request). */ - public Map>> mappings() { + public Map> mappings() { return mappings; } @@ -122,32 +130,23 @@ public Map>> mappings() { * @param field field name as specified in the {@link GetFieldMappingsRequest} * @return FieldMappingMetaData for the requested field or null if not found. */ - public FieldMappingMetaData fieldMappings(String index, String type, String field) { - Map> indexMapping = mappings.get(index); + public FieldMappingMetaData fieldMappings(String index, String field) { + Map indexMapping = mappings.get(index); if (indexMapping == null) { return null; } - Map typeMapping = indexMapping.get(type); - if (typeMapping == null) { - return null; - } - return typeMapping.get(field); + return indexMapping.get(field); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - for (Map.Entry>> indexEntry : mappings.entrySet()) { + for (Map.Entry> indexEntry : mappings.entrySet()) { builder.startObject(indexEntry.getKey()); builder.startObject(MAPPINGS.getPreferredName()); - Map mappings = null; - for (Map.Entry> typeEntry : indexEntry.getValue().entrySet()) { - assert mappings == null; - mappings = typeEntry.getValue(); - } - if (mappings != null) { - addFieldMappingsToBuilder(builder, params, mappings); + if (indexEntry.getValue() != null) { + addFieldMappingsToBuilder(builder, params, indexEntry.getValue()); } builder.endObject(); @@ -255,13 +254,22 @@ public int hashCode() { @Override public void writeTo(StreamOutput out) throws IOException { out.writeVInt(mappings.size()); - for (Map.Entry>> indexEntry : mappings.entrySet()) { + for (Map.Entry> indexEntry : mappings.entrySet()) { out.writeString(indexEntry.getKey()); - out.writeVInt(indexEntry.getValue().size()); - for (Map.Entry> typeEntry : indexEntry.getValue().entrySet()) { - out.writeString(typeEntry.getKey()); - out.writeVInt(typeEntry.getValue().size()); - for (Map.Entry fieldEntry : typeEntry.getValue().entrySet()) { + if (out.getVersion().before(Version.V_8_0_0)) { + out.writeVInt(1); + out.writeString(MapperService.SINGLE_MAPPING_NAME); + out.writeVInt(indexEntry.getValue().size()); + for (Map.Entry fieldEntry : indexEntry.getValue().entrySet()) { + out.writeString(fieldEntry.getKey()); + FieldMappingMetaData fieldMapping = fieldEntry.getValue(); + out.writeString(fieldMapping.fullName()); + out.writeBytesReference(fieldMapping.source); + } + + } else { + out.writeVInt(indexEntry.getValue().size()); + for (Map.Entry fieldEntry : indexEntry.getValue().entrySet()) { out.writeString(fieldEntry.getKey()); FieldMappingMetaData fieldMapping = fieldEntry.getValue(); out.writeString(fieldMapping.fullName()); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java index c95435834b499..8deae3d686e79 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java @@ -65,7 +65,7 @@ protected void doExecute(Task task, GetFieldMappingsRequest request, final Actio if (concreteIndices.length == 0) { listener.onResponse(new GetFieldMappingsResponse(emptyMap())); } else { - boolean probablySingleFieldRequest = concreteIndices.length == 1 && request.types().length == 1 && request.fields().length == 1; + boolean probablySingleFieldRequest = concreteIndices.length == 1 && request.fields().length == 1; for (final String index : concreteIndices) { GetFieldMappingsIndexRequest shardRequest = new GetFieldMappingsIndexRequest(request, index, probablySingleFieldRequest); @@ -92,7 +92,7 @@ public void onFailure(Exception e) { } private GetFieldMappingsResponse merge(AtomicReferenceArray indexResponses) { - Map>> mergedResponses = new HashMap<>(); + Map> mergedResponses = new HashMap<>(); for (int i = 0; i < indexResponses.length(); i++) { Object element = indexResponses.get(i); if (element instanceof GetFieldMappingsResponse) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java index 842f00d59c6d0..84e6237635a2c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java @@ -44,12 +44,10 @@ import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.IndicesService; -import org.elasticsearch.indices.TypeMissingException; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import java.io.IOException; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -99,30 +97,9 @@ protected GetFieldMappingsResponse shardOperation(final GetFieldMappingsIndexReq Predicate metadataFieldPredicate = (f) -> indicesService.isMetaDataField(indexCreatedVersion, f); Predicate fieldPredicate = metadataFieldPredicate.or(indicesService.getFieldFilter().apply(shardId.getIndexName())); - DocumentMapper mapper = indexService.mapperService().documentMapper(); - Collection typeIntersection; - if (request.types().length == 0) { - typeIntersection = mapper == null - ? Collections.emptySet() - : Collections.singleton(mapper.type()); - } else { - typeIntersection = mapper != null && Regex.simpleMatch(request.types(), mapper.type()) - ? Collections.singleton(mapper.type()) - : Collections.emptySet(); - if (typeIntersection.isEmpty()) { - throw new TypeMissingException(shardId.getIndex(), request.types()); - } - } - - Map> typeMappings = new HashMap<>(); - for (String type : typeIntersection) { - DocumentMapper documentMapper = indexService.mapperService().documentMapper(); - Map fieldMapping = findFieldMappingsByType(fieldPredicate, documentMapper, request); - if (!fieldMapping.isEmpty()) { - typeMappings.put(type, fieldMapping); - } - } - return new GetFieldMappingsResponse(singletonMap(shardId.getIndexName(), Collections.unmodifiableMap(typeMappings))); + DocumentMapper documentMapper = indexService.mapperService().documentMapper(); + Map fieldMapping = findFieldMappings(fieldPredicate, documentMapper, request); + return new GetFieldMappingsResponse(singletonMap(shardId.getIndexName(), fieldMapping)); } @Override @@ -172,9 +149,12 @@ public Boolean paramAsBoolean(String key, Boolean defaultValue) { } }; - private static Map findFieldMappingsByType(Predicate fieldPredicate, + private static Map findFieldMappings(Predicate fieldPredicate, DocumentMapper documentMapper, GetFieldMappingsIndexRequest request) { + if (documentMapper == null) { + return Collections.emptyMap(); + } Map fieldMappings = new HashMap<>(); final DocumentFieldMappers allFieldMappers = documentMapper.mappers(); for (String field : request.fields()) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java index 2f9cd698cf3fb..af24b3b130157 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java @@ -66,7 +66,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC client.admin().indices().getFieldMappings(getMappingsRequest, new RestBuilderListener<>(channel) { @Override public RestResponse buildResponse(GetFieldMappingsResponse response, XContentBuilder builder) throws Exception { - Map>> mappingsByIndex = response.mappings(); + Map> mappingsByIndex = response.mappings(); boolean isPossibleSingleFieldRequest = indices.length == 1 && fields.length == 1; if (isPossibleSingleFieldRequest && isFieldMappingMissingField(mappingsByIndex)) { @@ -85,22 +85,21 @@ public RestResponse buildResponse(GetFieldMappingsResponse response, XContentBui /** * Helper method to find out if the only included fieldmapping metadata is typed NULL, which means - * that type and index exist, but the field did not + * that the index exists, but the field did not */ - private boolean isFieldMappingMissingField(Map>> mappingsByIndex) { + private boolean isFieldMappingMissingField(Map> mappingsByIndex) { if (mappingsByIndex.size() != 1) { return false; } - for (Map> value : mappingsByIndex.values()) { - for (Map fieldValue : value.values()) { - for (Map.Entry fieldMappingMetaDataEntry : fieldValue.entrySet()) { - if (fieldMappingMetaDataEntry.getValue().isNull()) { - return true; - } + for (Map fieldValue : mappingsByIndex.values()) { + for (Map.Entry fieldMappingMetaDataEntry : fieldValue.entrySet()) { + if (fieldMappingMetaDataEntry.getValue().isNull()) { + return true; } } } + return false; } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java index 2e356c06b6eee..d15b9841c4dd0 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java @@ -34,16 +34,16 @@ public class GetFieldMappingsResponseTests extends AbstractWireSerializingTestCase { public void testManualSerialization() throws IOException { - Map>> mappings = new HashMap<>(); + Map> mappings = new HashMap<>(); FieldMappingMetaData fieldMappingMetaData = new FieldMappingMetaData("my field", new BytesArray("{}")); - mappings.put("index", Collections.singletonMap("type", Collections.singletonMap("field", fieldMappingMetaData))); + mappings.put("index", Collections.singletonMap("field", fieldMappingMetaData)); GetFieldMappingsResponse response = new GetFieldMappingsResponse(mappings); try (BytesStreamOutput out = new BytesStreamOutput()) { response.writeTo(out); try (StreamInput in = StreamInput.wrap(out.bytes().toBytesRef().bytes)) { GetFieldMappingsResponse serialized = new GetFieldMappingsResponse(in); - FieldMappingMetaData metaData = serialized.fieldMappings("index", "type", "field"); + FieldMappingMetaData metaData = serialized.fieldMappings("index", "field"); assertNotNull(metaData); assertEquals(new BytesArray("{}"), metaData.getSource()); } @@ -60,25 +60,20 @@ protected Writeable.Reader instanceReader() { return GetFieldMappingsResponse::new; } - private Map>> randomMapping() { - Map>> mappings = new HashMap<>(); + private Map> randomMapping() { + Map> mappings = new HashMap<>(); int indices = randomInt(10); for(int i = 0; i < indices; i++) { - final Map> doctypesMappings = new HashMap<>(); - int doctypes = randomInt(10); - for(int j = 0; j < doctypes; j++) { - Map fieldMappings = new HashMap<>(); - int fields = randomInt(10); - for(int k = 0; k < fields; k++) { - final String mapping = randomBoolean() ? "{\"type\":\"string\"}" : "{\"type\":\"keyword\"}"; - FieldMappingMetaData metaData = - new FieldMappingMetaData("my field", new BytesArray(mapping)); - fieldMappings.put("field" + k, metaData); - } - doctypesMappings.put("doctype" + j, fieldMappings); + Map fieldMappings = new HashMap<>(); + int fields = randomInt(10); + for (int k = 0; k < fields; k++) { + final String mapping = randomBoolean() ? "{\"type\":\"string\"}" : "{\"type\":\"keyword\"}"; + FieldMappingMetaData metaData = + new FieldMappingMetaData("my field", new BytesArray(mapping)); + fieldMappings.put("field" + k, metaData); } - mappings.put("index" + i, doctypesMappings); + mappings.put("index" + i, fieldMappings); } return mappings; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java index fd8c91bb1e55e..e10d0c6f5f889 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java @@ -78,7 +78,7 @@ public void testGetIndex() { public void testGetFieldMappings() { GetFieldMappingsResponse getFieldMappingsResponse = client().admin().indices().prepareGetFieldMappings().setFields("*").get(); - Map>> mappings = getFieldMappingsResponse.mappings(); + Map> mappings = getFieldMappingsResponse.mappings(); assertEquals(2, mappings.size()); assertFieldMappings(mappings.get("index1"), ALL_FLAT_FIELDS); assertFieldMappings(mappings.get("filtered"), FILTERED_FLAT_FIELDS); @@ -126,10 +126,8 @@ private static void assertFieldCaps(FieldCapabilitiesResponse fieldCapabilitiesR assertEquals("Some unexpected fields were returned: " + responseMap.keySet(), 0, responseMap.size()); } - private static void assertFieldMappings(Map> mappings, + private static void assertFieldMappings(Map fields, Collection expectedFields) { - assertEquals(1, mappings.size()); - Map fields = new HashMap<>(mappings.get("_doc")); Set builtInMetaDataFields = IndicesModule.getBuiltInMetaDataFields(); for (String field : builtInMetaDataFields) { GetFieldMappingsResponse.FieldMappingMetaData fieldMappingMetaData = fields.remove(field); diff --git a/server/src/test/java/org/elasticsearch/indices/mapping/SimpleGetFieldMappingsIT.java b/server/src/test/java/org/elasticsearch/indices/mapping/SimpleGetFieldMappingsIT.java index e344f15f3c55f..cf7b0f1bebefd 100644 --- a/server/src/test/java/org/elasticsearch/indices/mapping/SimpleGetFieldMappingsIT.java +++ b/server/src/test/java/org/elasticsearch/indices/mapping/SimpleGetFieldMappingsIT.java @@ -50,7 +50,6 @@ import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; public class SimpleGetFieldMappingsIT extends ESIntegTestCase { @@ -65,7 +64,7 @@ public void testGetMappingsWhereThereAreNone() { assertThat(response.mappings().size(), equalTo(1)); assertThat(response.mappings().get("index").size(), equalTo(0)); - assertThat(response.fieldMappings("index", "type", "field"), Matchers.nullValue()); + assertThat(response.fieldMappings("index", "field"), Matchers.nullValue()); } private XContentBuilder getMappingForType(String type) throws IOException { @@ -100,48 +99,28 @@ public void testGetFieldMappings() throws Exception { // Get mappings by full name - GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings("indexa").setTypes("typeA") + GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings("indexa") .setFields("field1", "obj.subfield").get(); - assertThat(response.fieldMappings("indexa", "typeA", "field1").fullName(), equalTo("field1")); - assertThat(response.fieldMappings("indexa", "typeA", "field1").sourceAsMap(), hasKey("field1")); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").fullName(), equalTo("obj.subfield")); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").sourceAsMap(), hasKey("subfield")); - assertThat(response.fieldMappings("indexb", "typeB", "field1"), nullValue()); + assertThat(response.fieldMappings("indexa", "field1").fullName(), equalTo("field1")); + assertThat(response.fieldMappings("indexa", "field1").sourceAsMap(), hasKey("field1")); + assertThat(response.fieldMappings("indexa", "obj.subfield").fullName(), equalTo("obj.subfield")); + assertThat(response.fieldMappings("indexa", "obj.subfield").sourceAsMap(), hasKey("subfield")); // Get mappings by name - response = client().admin().indices().prepareGetFieldMappings("indexa").setTypes("typeA").setFields("field1", "obj.subfield") + response = client().admin().indices().prepareGetFieldMappings("indexa").setFields("field1", "obj.subfield") .get(); - assertThat(response.fieldMappings("indexa", "typeA", "field1").fullName(), equalTo("field1")); - assertThat(response.fieldMappings("indexa", "typeA", "field1").sourceAsMap(), hasKey("field1")); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").fullName(), equalTo("obj.subfield")); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").sourceAsMap(), hasKey("subfield")); - assertThat(response.fieldMappings("indexa", "typeB", "field1"), nullValue()); - assertThat(response.fieldMappings("indexb", "typeB", "field1"), nullValue()); + assertThat(response.fieldMappings("indexa", "field1").fullName(), equalTo("field1")); + assertThat(response.fieldMappings("indexa", "field1").sourceAsMap(), hasKey("field1")); + assertThat(response.fieldMappings("indexa", "obj.subfield").fullName(), equalTo("obj.subfield")); + assertThat(response.fieldMappings("indexa", "obj.subfield").sourceAsMap(), hasKey("subfield")); // get mappings by name across multiple indices - response = client().admin().indices().prepareGetFieldMappings().setTypes("typeA").setFields("obj.subfield").get(); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").fullName(), equalTo("obj.subfield")); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").sourceAsMap(), hasKey("subfield")); - assertThat(response.fieldMappings("indexa", "typeB", "obj.subfield"), nullValue()); - assertThat(response.fieldMappings("indexb", "typeB", "obj.subfield"), nullValue()); - - // get mappings by name across multiple types - response = client().admin().indices().prepareGetFieldMappings("indexa").setFields("obj.subfield").get(); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").fullName(), equalTo("obj.subfield")); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").sourceAsMap(), hasKey("subfield")); - assertThat(response.fieldMappings("indexa", "typeA", "field1"), nullValue()); - assertThat(response.fieldMappings("indexb", "typeB", "obj.subfield"), nullValue()); - assertThat(response.fieldMappings("indexb", "typeB", "field1"), nullValue()); - - // get mappings by name across multiple types & indices response = client().admin().indices().prepareGetFieldMappings().setFields("obj.subfield").get(); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").fullName(), equalTo("obj.subfield")); - assertThat(response.fieldMappings("indexa", "typeA", "obj.subfield").sourceAsMap(), hasKey("subfield")); - assertThat(response.fieldMappings("indexa", "typeA", "field1"), nullValue()); - assertThat(response.fieldMappings("indexb", "typeB", "field1"), nullValue()); - assertThat(response.fieldMappings("indexb", "typeB", "obj.subfield").fullName(), equalTo("obj.subfield")); - assertThat(response.fieldMappings("indexb", "typeB", "obj.subfield").sourceAsMap(), hasKey("subfield")); - assertThat(response.fieldMappings("indexb", "typeB", "field1"), nullValue()); + assertThat(response.fieldMappings("indexa", "obj.subfield").fullName(), equalTo("obj.subfield")); + assertThat(response.fieldMappings("indexa", "obj.subfield").sourceAsMap(), hasKey("subfield")); + assertThat(response.fieldMappings("indexb", "obj.subfield").fullName(), equalTo("obj.subfield")); + assertThat(response.fieldMappings("indexb", "obj.subfield").sourceAsMap(), hasKey("subfield")); + } @SuppressWarnings("unchecked") @@ -152,15 +131,15 @@ public void testSimpleGetFieldMappingsWithDefaults() throws Exception { GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings() .setFields("num", "field1", "obj.subfield").includeDefaults(true).get(); - assertThat((Map) response.fieldMappings("test", "type", "num").sourceAsMap().get("num"), + assertThat((Map) response.fieldMappings("test", "num").sourceAsMap().get("num"), hasEntry("index", Boolean.TRUE)); - assertThat((Map) response.fieldMappings("test", "type", "num").sourceAsMap().get("num"), + assertThat((Map) response.fieldMappings("test", "num").sourceAsMap().get("num"), hasEntry("type", "long")); - assertThat((Map) response.fieldMappings("test", "type", "field1").sourceAsMap().get("field1"), + assertThat((Map) response.fieldMappings("test", "field1").sourceAsMap().get("field1"), hasEntry("index", Boolean.TRUE)); - assertThat((Map) response.fieldMappings("test", "type", "field1").sourceAsMap().get("field1"), + assertThat((Map) response.fieldMappings("test", "field1").sourceAsMap().get("field1"), hasEntry("type", "text")); - assertThat((Map) response.fieldMappings("test", "type", "obj.subfield").sourceAsMap().get("subfield"), + assertThat((Map) response.fieldMappings("test", "obj.subfield").sourceAsMap().get("subfield"), hasEntry("type", "keyword")); } @@ -171,12 +150,12 @@ public void testGetFieldMappingsWithFieldAlias() throws Exception { GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings() .setFields("alias", "field1").get(); - FieldMappingMetaData aliasMapping = response.fieldMappings("test", "type", "alias"); + FieldMappingMetaData aliasMapping = response.fieldMappings("test", "alias"); assertThat(aliasMapping.fullName(), equalTo("alias")); assertThat(aliasMapping.sourceAsMap(), hasKey("alias")); assertThat((Map) aliasMapping.sourceAsMap().get("alias"), hasEntry("type", "alias")); - FieldMappingMetaData field1Mapping = response.fieldMappings("test", "type", "field1"); + FieldMappingMetaData field1Mapping = response.fieldMappings("test", "field1"); assertThat(field1Mapping.fullName(), equalTo("field1")); assertThat(field1Mapping.sourceAsMap(), hasKey("field1")); } @@ -187,7 +166,7 @@ public void testSimpleGetFieldMappingsWithPretty() throws Exception { Map params = new HashMap<>(); params.put("pretty", "true"); GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings("index") - .setTypes("type").setFields("field1", "obj.subfield").get(); + .setFields("field1", "obj.subfield").get(); XContentBuilder responseBuilder = XContentFactory.jsonBuilder().prettyPrint(); response.toXContent(responseBuilder, new ToXContent.MapParams(params)); String responseStrings = Strings.toString(responseBuilder); @@ -200,7 +179,7 @@ public void testSimpleGetFieldMappingsWithPretty() throws Exception { params.put("pretty", "false"); response = client().admin().indices().prepareGetFieldMappings("index") - .setTypes("type").setFields("field1", "obj.subfield").get(); + .setFields("field1", "obj.subfield").get(); responseBuilder = XContentFactory.jsonBuilder().prettyPrint().lfAtEnd(); response.toXContent(responseBuilder, new ToXContent.MapParams(params)); responseStrings = Strings.toString(responseBuilder); @@ -218,9 +197,9 @@ public void testGetFieldMappingsWithBlocks() throws Exception { for (String block : Arrays.asList(SETTING_BLOCKS_READ, SETTING_BLOCKS_WRITE, SETTING_READ_ONLY)) { try { enableIndexBlock("test", block); - GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings("test").setTypes("_doc") + GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings("test") .setFields("field1", "obj.subfield").get(); - assertThat(response.fieldMappings("test", "_doc", "field1").fullName(), equalTo("field1")); + assertThat(response.fieldMappings("test", "field1").fullName(), equalTo("field1")); } finally { disableIndexBlock("test", block); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java index 1d75e0f1883e0..1eaab99af85ab 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java @@ -319,7 +319,7 @@ public void testGetFieldMappingsIsFiltered() { Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) .admin().indices().prepareGetFieldMappings("test").setFields("*").get(); - Map>> mappings = + Map> mappings = getFieldMappingsResponse.mappings(); assertEquals(1, mappings.size()); assertExpectedFields(mappings.get("test"), "field1"); @@ -329,7 +329,7 @@ public void testGetFieldMappingsIsFiltered() { Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) .admin().indices().prepareGetFieldMappings("test").setFields("*").get(); - Map>> mappings = + Map> mappings = getFieldMappingsResponse.mappings(); assertEquals(1, mappings.size()); assertExpectedFields(mappings.get("test"), "field2"); @@ -339,7 +339,7 @@ public void testGetFieldMappingsIsFiltered() { Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user3", USERS_PASSWD))) .admin().indices().prepareGetFieldMappings("test").setFields("*").get(); - Map>> mappings = + Map> mappings = getFieldMappingsResponse.mappings(); assertEquals(1, mappings.size()); assertExpectedFields(mappings.get("test"), "field1"); @@ -349,7 +349,7 @@ public void testGetFieldMappingsIsFiltered() { Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user4", USERS_PASSWD))) .admin().indices().prepareGetFieldMappings("test").setFields("*").get(); - Map>> mappings = + Map> mappings = getFieldMappingsResponse.mappings(); assertEquals(1, mappings.size()); assertExpectedFields(mappings.get("test"), "field1", "field2"); @@ -423,10 +423,8 @@ private static void assertExpectedFields(FieldCapabilitiesResponse fieldCapabili assertEquals("Some unexpected fields were returned: " + responseMap.keySet(), 0, responseMap.size()); } - private static void assertExpectedFields(Map> mappings, + private static void assertExpectedFields(Map fields, String... expectedFields) { - assertEquals(1, mappings.size()); - Map fields = new HashMap<>(mappings.get("type1")); Set builtInMetaDataFields = IndicesModule.getBuiltInMetaDataFields(); for (String field : builtInMetaDataFields) { GetFieldMappingsResponse.FieldMappingMetaData fieldMappingMetaData = fields.remove(field); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java index e99abb225defa..9ca59b554d9e2 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java @@ -61,13 +61,12 @@ public String configUsersRoles() { public void testFieldMappings() throws Exception { final String index = "logstash-20-12-2015"; - final String type = "_doc"; final String field = "foo"; indexRandom(true, client().prepareIndex().setIndex(index).setSource(field, "bar")); GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings().addIndices("logstash-*").setFields("*") .includeDefaults(true).get(); - FieldMappingMetaData fieldMappingMetaData = response.fieldMappings(index, type, field); + FieldMappingMetaData fieldMappingMetaData = response.fieldMappings(index, field); assertThat(fieldMappingMetaData, notNullValue()); assertThat(fieldMappingMetaData.isNull(), is(false)); @@ -76,7 +75,7 @@ public void testFieldMappings() throws Exception { .admin().indices().prepareGetFieldMappings().addIndices("logstash-*") .setFields("*") .includeDefaults(true).get(); - FieldMappingMetaData fieldMappingMetaData1 = response.fieldMappings(index, type, field); + FieldMappingMetaData fieldMappingMetaData1 = response.fieldMappings(index, field); assertThat(fieldMappingMetaData1, notNullValue()); assertThat(fieldMappingMetaData1.isNull(), is(false)); assertThat(fieldMappingMetaData1.fullName(), equalTo(fieldMappingMetaData.fullName())); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java index 11a379546b310..5134337795b74 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java @@ -17,7 +17,6 @@ import java.util.stream.Collectors; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; -import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME; import static org.elasticsearch.search.builder.SearchSourceBuilder.searchSource; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.watcher.actions.ActionBuilders.loggingAction; @@ -77,13 +76,11 @@ public void testTransformFields() throws Exception { GetFieldMappingsResponse response = client().admin().indices() .prepareGetFieldMappings(".watcher-history*") .setFields("result.actions.transform.payload") - .setTypes(SINGLE_MAPPING_NAME) .includeDefaults(true) .get(); // time might have rolled over to a new day, thus we need to check that this field exists only in one of the history indices List payloadNulls = response.mappings().values().stream() - .map(map -> map.get(SINGLE_MAPPING_NAME)) .map(map -> map.get("result.actions.transform.payload")) .filter(Objects::nonNull) .map(GetFieldMappingsResponse.FieldMappingMetaData::isNull) From f660bf3840997d96e4005e960612f1baf46fc9b5 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 11 Nov 2019 10:47:22 +0000 Subject: [PATCH 2/8] feedback; tests --- .../api/indices.get_field_mapping.json | 4 - .../20_missing_field.yml | 4 +- .../mapping/get/GetFieldMappingsRequest.java | 6 +- .../mapping/get/GetFieldMappingsResponse.java | 74 ++++--------------- .../mapping/get/GetMappingsResponse.java | 4 +- .../mapper/FieldFilterMapperPluginTests.java | 12 ++- .../DocumentAndFieldLevelSecurityTests.java | 3 +- 7 files changed, 37 insertions(+), 70 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_field_mapping.json b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_field_mapping.json index 9b210b893275e..4c5191f2f320a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_field_mapping.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_field_mapping.json @@ -38,10 +38,6 @@ ] }, "params":{ - "include_type_name":{ - "type":"boolean", - "description":"Whether a type should be returned in the body of the mappings." - }, "include_defaults":{ "type":"boolean", "description":"Whether the default mapping values should be returned as well" diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml index 77b795686db4e..708fdd5367266 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml @@ -1,5 +1,5 @@ --- -"Return empty object if field doesn't exist, but type and index do": +"Return empty object if field doesn't exist, but index does": - do: indices.create: @@ -16,4 +16,4 @@ index: test_index fields: not_existent - - match: { 'test_index.mappings': {}} + - match: { '': {}} diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsRequest.java index 61bf93f7bf96d..af0c821fdd8e3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsRequest.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; +import java.util.Arrays; /** * Request the mappings of specific fields @@ -54,7 +55,10 @@ public GetFieldMappingsRequest(StreamInput in) throws IOException { super(in); indices = in.readStringArray(); if (in.getVersion().before(Version.V_8_0_0)) { - in.readStringArray(); + String[] types = in.readStringArray(); + if (types != Strings.EMPTY_ARRAY) { + throw new IllegalArgumentException("Expected empty type array but received [" + Arrays.toString(types) + "]"); + } } indicesOptions = IndicesOptions.readIndicesOptions(in); local = in.readBoolean(); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java index 11778d1ee7a31..0c6fc1cb4cf04 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java @@ -57,33 +57,6 @@ public class GetFieldMappingsResponse extends ActionResponse implements ToXConte private static final ParseField MAPPINGS = new ParseField("mappings"); - private static final ObjectParser>, String> PARSER = - new ObjectParser<>(MAPPINGS.getPreferredName(), true, HashMap::new); - - static { - PARSER.declareField((p, typeMappings, index) -> { - p.nextToken(); - while (p.currentToken() == XContentParser.Token.FIELD_NAME) { - final String typeName = p.currentName(); - - if (p.nextToken() == XContentParser.Token.START_OBJECT) { - final Map typeMapping = new HashMap<>(); - typeMappings.put(typeName, typeMapping); - - while (p.nextToken() == XContentParser.Token.FIELD_NAME) { - final String fieldName = p.currentName(); - final FieldMappingMetaData fieldMappingMetaData = FieldMappingMetaData.fromXContent(p); - typeMapping.put(fieldName, fieldMappingMetaData); - } - } else { - p.skipChildren(); - } - p.nextToken(); - } - }, MAPPINGS, ObjectParser.ValueType.OBJECT); - } - - // TODO remove the middle `type` level of this private final Map> mappings; GetFieldMappingsResponse(Map> mappings) { @@ -96,30 +69,22 @@ public class GetFieldMappingsResponse extends ActionResponse implements ToXConte Map> indexMapBuilder = new HashMap<>(size); for (int i = 0; i < size; i++) { String index = in.readString(); - Map fieldMapBuilder = new HashMap<>(); if (in.getVersion().before(Version.V_8_0_0)) { int typesSize = in.readVInt(); - assert typesSize <= 1; - for (int j = 0; j < typesSize; j++) { - in.readString(); // type - int fieldSize = in.readVInt(); - for (int k = 0; k < fieldSize; k++) { - fieldMapBuilder.put(in.readString(), new FieldMappingMetaData(in.readString(), in.readBytesReference())); - } - } - } else { - int fieldSize = in.readVInt(); - for (int k = 0; k < fieldSize; k++) { - fieldMapBuilder.put(in.readString(), new FieldMappingMetaData(in.readString(), in.readBytesReference())); - } + assert typesSize == 1; + in.readString(); // type + } + int fieldSize = in.readVInt(); + Map fieldMapBuilder = new HashMap<>(fieldSize); + for (int k = 0; k < fieldSize; k++) { + fieldMapBuilder.put(in.readString(), new FieldMappingMetaData(in.readString(), in.readBytesReference())); } indexMapBuilder.put(index, unmodifiableMap(fieldMapBuilder)); } mappings = unmodifiableMap(indexMapBuilder); - } - /** returns the retrieved field mapping. The return map keys are index, type, field (as specified in the request). */ + /** returns the retrieved field mapping. The return map keys are index, field (as specified in the request). */ public Map> mappings() { return mappings; } @@ -259,22 +224,13 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().before(Version.V_8_0_0)) { out.writeVInt(1); out.writeString(MapperService.SINGLE_MAPPING_NAME); - out.writeVInt(indexEntry.getValue().size()); - for (Map.Entry fieldEntry : indexEntry.getValue().entrySet()) { - out.writeString(fieldEntry.getKey()); - FieldMappingMetaData fieldMapping = fieldEntry.getValue(); - out.writeString(fieldMapping.fullName()); - out.writeBytesReference(fieldMapping.source); - } - - } else { - out.writeVInt(indexEntry.getValue().size()); - for (Map.Entry fieldEntry : indexEntry.getValue().entrySet()) { - out.writeString(fieldEntry.getKey()); - FieldMappingMetaData fieldMapping = fieldEntry.getValue(); - out.writeString(fieldMapping.fullName()); - out.writeBytesReference(fieldMapping.source); - } + } + out.writeVInt(indexEntry.getValue().size()); + for (Map.Entry fieldEntry : indexEntry.getValue().entrySet()) { + out.writeString(fieldEntry.getKey()); + FieldMappingMetaData fieldMapping = fieldEntry.getValue(); + out.writeString(fieldMapping.fullName()); + out.writeBytesReference(fieldMapping.source); } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponse.java index 30b939d627e38..61e84147e3d53 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetMappingsResponse.java @@ -99,13 +99,13 @@ public void writeTo(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { for (final ObjectObjectCursor indexEntry : getMappings()) { + builder.startObject(indexEntry.key); if (indexEntry.value != null) { - builder.startObject(indexEntry.key); builder.field(MAPPINGS.getPreferredName(), indexEntry.value.sourceAsMap()); - builder.endObject(); } else { builder.startObject(MAPPINGS.getPreferredName()).endObject(); } + builder.endObject(); } return builder; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java index e10d0c6f5f889..6bc443ed0efa3 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java @@ -92,6 +92,15 @@ public void testGetFieldMappings() { assertFieldMappings(response.mappings().get("test"), FILTERED_FLAT_FIELDS); } + public void testGetNonExistentFieldMapping() { + GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings("index1").setFields("non-existent").get(); + Map> mappings = response.mappings(); + assertEquals(1, mappings.size()); + Map fieldmapping = mappings.get("index1"); + assertEquals(1, fieldmapping.size()); + assertEquals(GetFieldMappingsResponse.FieldMappingMetaData.NULL, fieldmapping.get("non-existent")); + } + public void testFieldCapabilities() { List allFields = new ArrayList<>(ALL_FLAT_FIELDS); allFields.addAll(ALL_OBJECT_FIELDS); @@ -126,9 +135,10 @@ private static void assertFieldCaps(FieldCapabilitiesResponse fieldCapabilitiesR assertEquals("Some unexpected fields were returned: " + responseMap.keySet(), 0, responseMap.size()); } - private static void assertFieldMappings(Map fields, + private static void assertFieldMappings(Map actual, Collection expectedFields) { Set builtInMetaDataFields = IndicesModule.getBuiltInMetaDataFields(); + Map fields = new HashMap<>(actual); for (String field : builtInMetaDataFields) { GetFieldMappingsResponse.FieldMappingMetaData fieldMappingMetaData = fields.remove(field); assertNotNull(" expected field [" + field + "] not found", fieldMappingMetaData); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java index 1eaab99af85ab..0a28440749264 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/DocumentAndFieldLevelSecurityTests.java @@ -423,9 +423,10 @@ private static void assertExpectedFields(FieldCapabilitiesResponse fieldCapabili assertEquals("Some unexpected fields were returned: " + responseMap.keySet(), 0, responseMap.size()); } - private static void assertExpectedFields(Map fields, + private static void assertExpectedFields(Map actual, String... expectedFields) { Set builtInMetaDataFields = IndicesModule.getBuiltInMetaDataFields(); + Map fields = new HashMap<>(actual); for (String field : builtInMetaDataFields) { GetFieldMappingsResponse.FieldMappingMetaData fieldMappingMetaData = fields.remove(field); assertNotNull(" expected field [" + field + "] not found", fieldMappingMetaData); From cc2af674d4d25a747228230ab3005659dc6c175d Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 12 Nov 2019 17:17:48 +0000 Subject: [PATCH 3/8] Remove special-case when no fields are found --- .../test/indices.get_field_mapping/20_missing_field.yml | 2 +- .../action/admin/indices/RestGetFieldMappingAction.java | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml index 708fdd5367266..4b42e41276b44 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml @@ -16,4 +16,4 @@ index: test_index fields: not_existent - - match: { '': {}} + - match: { 'test_index.mappings': {}} diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java index af24b3b130157..32e556cf2268c 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java @@ -67,12 +67,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC @Override public RestResponse buildResponse(GetFieldMappingsResponse response, XContentBuilder builder) throws Exception { Map> mappingsByIndex = response.mappings(); - - boolean isPossibleSingleFieldRequest = indices.length == 1 && fields.length == 1; - if (isPossibleSingleFieldRequest && isFieldMappingMissingField(mappingsByIndex)) { - return new BytesRestResponse(OK, builder.startObject().endObject()); - } - + RestStatus status = OK; if (mappingsByIndex.isEmpty() && fields.length > 0) { status = NOT_FOUND; From 7e8a493eed4d4b1d7d3a05fb728c84aae7c52adb Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 13 Nov 2019 09:34:25 +0000 Subject: [PATCH 4/8] better tests for null mappings --- .../20_missing_field.yml | 1 + .../mapping/get/GetFieldMappingsResponse.java | 10 +++++--- .../indices/RestGetFieldMappingAction.java | 3 ++- .../get/GetFieldMappingsResponseTests.java | 25 +++++++++++++++++++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml index 4b42e41276b44..408d0742ab8a6 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get_field_mapping/20_missing_field.yml @@ -17,3 +17,4 @@ fields: not_existent - match: { 'test_index.mappings': {}} + diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java index 0c6fc1cb4cf04..b6d3495dd50b3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponse.java @@ -105,7 +105,7 @@ public FieldMappingMetaData fieldMappings(String index, String field) { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); + builder.startObject(); for (Map.Entry> indexEntry : mappings.entrySet()) { builder.startObject(indexEntry.getKey()); builder.startObject(MAPPINGS.getPreferredName()); @@ -125,9 +125,11 @@ private void addFieldMappingsToBuilder(XContentBuilder builder, Params params, Map mappings) throws IOException { for (Map.Entry fieldEntry : mappings.entrySet()) { - builder.startObject(fieldEntry.getKey()); - fieldEntry.getValue().toXContent(builder, params); - builder.endObject(); + if (fieldEntry.getValue().isNull() == false) { + builder.startObject(fieldEntry.getKey()); + fieldEntry.getValue().toXContent(builder, params); + builder.endObject(); + } } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java index 32e556cf2268c..1bb9d4edeb244 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java @@ -67,12 +67,13 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC @Override public RestResponse buildResponse(GetFieldMappingsResponse response, XContentBuilder builder) throws Exception { Map> mappingsByIndex = response.mappings(); - + RestStatus status = OK; if (mappingsByIndex.isEmpty() && fields.length > 0) { status = NOT_FOUND; } response.toXContent(builder, request); + logger.warn(Strings.toString(response)); return new BytesRestResponse(status, builder); } }); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java index d15b9841c4dd0..10b00653afa24 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsResponseTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.action.admin.indices.mapping.get; import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsResponse.FieldMappingMetaData; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; @@ -31,6 +32,8 @@ import java.util.HashMap; import java.util.Map; +import static org.hamcrest.CoreMatchers.containsString; + public class GetFieldMappingsResponseTests extends AbstractWireSerializingTestCase { public void testManualSerialization() throws IOException { @@ -50,6 +53,28 @@ public void testManualSerialization() throws IOException { } } + public void testNullFieldMappingToXContent() { + Map> mappings = new HashMap<>(); + mappings.put("index", Collections.singletonMap("field", FieldMappingMetaData.NULL)); + GetFieldMappingsResponse response = new GetFieldMappingsResponse(mappings); + assertEquals("{\"index\":{\"mappings\":{}}}", Strings.toString(response)); + } + + public void testMixedNullAndPresentFieldMappingToXContent() { + Map> mappings = new HashMap<>(); + mappings.put("index", Map.of("field", FieldMappingMetaData.NULL, + "field1", new FieldMappingMetaData("field1", new BytesArray("{\"type\":\"string\"}")))); + mappings.put("index2", Map.of("field", new FieldMappingMetaData("field", new BytesArray("{}")))); + mappings.put("index3", Map.of("field", FieldMappingMetaData.NULL)); + GetFieldMappingsResponse response = new GetFieldMappingsResponse(mappings); + String respAsString = Strings.toString(response); + assertThat(respAsString, containsString("\"index3\":{\"mappings\":{}}")); + assertThat(respAsString, + containsString("\"index\":{\"mappings\":{\"field1\":{\"full_name\":\"field1\",\"mapping\":{\"type\":\"string\"}}}}")); + assertThat(respAsString, + containsString("\"index2\":{\"mappings\":{\"field\":{\"full_name\":\"field\",\"mapping\":{}}}}")); + } + @Override protected GetFieldMappingsResponse createTestInstance() { return new GetFieldMappingsResponse(randomMapping()); From 57913520d2aafb63abd6b24506c074ecdc159a93 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 14 Nov 2019 13:27:04 +0000 Subject: [PATCH 5/8] cleanup --- .../indices/RestGetFieldMappingAction.java | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java index 1bb9d4edeb244..d4a5a0cb58140 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetFieldMappingAction.java @@ -73,29 +73,9 @@ public RestResponse buildResponse(GetFieldMappingsResponse response, XContentBui status = NOT_FOUND; } response.toXContent(builder, request); - logger.warn(Strings.toString(response)); return new BytesRestResponse(status, builder); } }); } - /** - * Helper method to find out if the only included fieldmapping metadata is typed NULL, which means - * that the index exists, but the field did not - */ - private boolean isFieldMappingMissingField(Map> mappingsByIndex) { - if (mappingsByIndex.size() != 1) { - return false; - } - - for (Map fieldValue : mappingsByIndex.values()) { - for (Map.Entry fieldMappingMetaDataEntry : fieldValue.entrySet()) { - if (fieldMappingMetaDataEntry.getValue().isNull()) { - return true; - } - } - } - - return false; - } } From 6a3847e047ed40aab92d25b589d9f0a73012597e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 22 Nov 2019 11:28:03 +0000 Subject: [PATCH 6/8] Remove special-case logic for existing index but non-existing field --- .../get/GetFieldMappingsIndexRequest.java | 16 +++++++--------- .../mapping/get/GetFieldMappingsResponse.java | 14 +++----------- .../get/TransportGetFieldMappingsAction.java | 3 +-- .../TransportGetFieldMappingsIndexAction.java | 2 -- .../get/GetFieldMappingsResponseTests.java | 19 +------------------ .../mapper/FieldFilterMapperPluginTests.java | 3 +-- 6 files changed, 13 insertions(+), 44 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java index b7dca8bde3455..fc6d58bb30a28 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/GetFieldMappingsIndexRequest.java @@ -32,7 +32,6 @@ public class GetFieldMappingsIndexRequest extends SingleShardRequest { - private final boolean probablySingleFieldRequest; private final boolean includeDefaults; private final String[] fields; @@ -45,12 +44,13 @@ public class GetFieldMappingsIndexRequest extends SingleShardRequest mappings) throws IOException { for (Map.Entry fieldEntry : mappings.entrySet()) { - if (fieldEntry.getValue().isNull() == false) { - builder.startObject(fieldEntry.getKey()); - fieldEntry.getValue().toXContent(builder, params); - builder.endObject(); - } + builder.startObject(fieldEntry.getKey()); + fieldEntry.getValue().toXContent(builder, params); + builder.endObject(); } } public static class FieldMappingMetaData implements ToXContentFragment { - public static final FieldMappingMetaData NULL = new FieldMappingMetaData("", BytesArray.EMPTY); private static final ParseField FULL_NAME = new ParseField("full_name"); private static final ParseField MAPPING = new ParseField("mapping"); @@ -172,10 +168,6 @@ public Map sourceAsMap() { return XContentHelper.convertToMap(source, true, XContentType.JSON).v2(); } - public boolean isNull() { - return NULL.fullName().equals(fullName) && NULL.source.length() == source.length(); - } - //pkg-private for testing BytesReference getSource() { return source; diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java index 8deae3d686e79..9f2124f390e3a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsAction.java @@ -65,9 +65,8 @@ protected void doExecute(Task task, GetFieldMappingsRequest request, final Actio if (concreteIndices.length == 0) { listener.onResponse(new GetFieldMappingsResponse(emptyMap())); } else { - boolean probablySingleFieldRequest = concreteIndices.length == 1 && request.fields().length == 1; for (final String index : concreteIndices) { - GetFieldMappingsIndexRequest shardRequest = new GetFieldMappingsIndexRequest(request, index, probablySingleFieldRequest); + GetFieldMappingsIndexRequest shardRequest = new GetFieldMappingsIndexRequest(request, index); client.executeLocally(TransportGetFieldMappingsIndexAction.TYPE, shardRequest, new ActionListener<>() { @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java index 84e6237635a2c..c7fe23b38d0e7 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/get/TransportGetFieldMappingsIndexAction.java @@ -174,8 +174,6 @@ private static Map findFieldMappings(Predicate { public void testManualSerialization() throws IOException { @@ -55,26 +53,11 @@ public void testManualSerialization() throws IOException { public void testNullFieldMappingToXContent() { Map> mappings = new HashMap<>(); - mappings.put("index", Collections.singletonMap("field", FieldMappingMetaData.NULL)); + mappings.put("index", Collections.emptyMap()); GetFieldMappingsResponse response = new GetFieldMappingsResponse(mappings); assertEquals("{\"index\":{\"mappings\":{}}}", Strings.toString(response)); } - public void testMixedNullAndPresentFieldMappingToXContent() { - Map> mappings = new HashMap<>(); - mappings.put("index", Map.of("field", FieldMappingMetaData.NULL, - "field1", new FieldMappingMetaData("field1", new BytesArray("{\"type\":\"string\"}")))); - mappings.put("index2", Map.of("field", new FieldMappingMetaData("field", new BytesArray("{}")))); - mappings.put("index3", Map.of("field", FieldMappingMetaData.NULL)); - GetFieldMappingsResponse response = new GetFieldMappingsResponse(mappings); - String respAsString = Strings.toString(response); - assertThat(respAsString, containsString("\"index3\":{\"mappings\":{}}")); - assertThat(respAsString, - containsString("\"index\":{\"mappings\":{\"field1\":{\"full_name\":\"field1\",\"mapping\":{\"type\":\"string\"}}}}")); - assertThat(respAsString, - containsString("\"index2\":{\"mappings\":{\"field\":{\"full_name\":\"field\",\"mapping\":{}}}}")); - } - @Override protected GetFieldMappingsResponse createTestInstance() { return new GetFieldMappingsResponse(randomMapping()); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java index baa2e69adfbe4..4b2d4228ab547 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java @@ -97,8 +97,7 @@ public void testGetNonExistentFieldMapping() { Map> mappings = response.mappings(); assertEquals(1, mappings.size()); Map fieldmapping = mappings.get("index1"); - assertEquals(1, fieldmapping.size()); - assertEquals(GetFieldMappingsResponse.FieldMappingMetaData.NULL, fieldmapping.get("non-existent")); + assertEquals(0, fieldmapping.size()); } public void testFieldCapabilities() { From 50bb69402c57bd32018216e0538db22b7df9d472 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 22 Nov 2019 11:43:38 +0000 Subject: [PATCH 7/8] plugin compilation --- .../integration/KibanaUserRoleIntegTests.java | 2 -- .../HistoryTemplateTransformMappingsTests.java | 11 ++++------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java index 9ca59b554d9e2..4e10bde26095b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/KibanaUserRoleIntegTests.java @@ -68,7 +68,6 @@ public void testFieldMappings() throws Exception { .includeDefaults(true).get(); FieldMappingMetaData fieldMappingMetaData = response.fieldMappings(index, field); assertThat(fieldMappingMetaData, notNullValue()); - assertThat(fieldMappingMetaData.isNull(), is(false)); response = client() .filterWithHeader(singletonMap("Authorization", UsernamePasswordToken.basicAuthHeaderValue("kibana_user", USERS_PASSWD))) @@ -77,7 +76,6 @@ public void testFieldMappings() throws Exception { .includeDefaults(true).get(); FieldMappingMetaData fieldMappingMetaData1 = response.fieldMappings(index, field); assertThat(fieldMappingMetaData1, notNullValue()); - assertThat(fieldMappingMetaData1.isNull(), is(false)); assertThat(fieldMappingMetaData1.fullName(), equalTo(fieldMappingMetaData.fullName())); } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java index 5134337795b74..de80c4891113e 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java @@ -12,9 +12,8 @@ import org.elasticsearch.xpack.core.watcher.transport.actions.put.PutWatchRequestBuilder; import org.elasticsearch.xpack.watcher.test.AbstractWatcherIntegrationTestCase; -import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; +import java.util.Optional; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.search.builder.SearchSourceBuilder.searchSource; @@ -26,7 +25,6 @@ import static org.elasticsearch.xpack.watcher.transform.TransformBuilders.searchTransform; import static org.elasticsearch.xpack.watcher.trigger.TriggerBuilders.schedule; import static org.elasticsearch.xpack.watcher.trigger.schedule.Schedules.interval; -import static org.hamcrest.Matchers.hasItem; public class HistoryTemplateTransformMappingsTests extends AbstractWatcherIntegrationTestCase { @@ -80,13 +78,12 @@ public void testTransformFields() throws Exception { .get(); // time might have rolled over to a new day, thus we need to check that this field exists only in one of the history indices - List payloadNulls = response.mappings().values().stream() + Optional mapping = response.mappings().values().stream() .map(map -> map.get("result.actions.transform.payload")) .filter(Objects::nonNull) - .map(GetFieldMappingsResponse.FieldMappingMetaData::isNull) - .collect(Collectors.toList()); + .findFirst(); - assertThat(payloadNulls, hasItem(true)); + assertTrue(mapping.isPresent()); }); } } From 0f137ddec8290bae7fb118145b0a59414283fa92 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 22 Nov 2019 13:30:28 +0000 Subject: [PATCH 8/8] watcher test --- .../watcher/history/HistoryTemplateTransformMappingsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java index de80c4891113e..2c7524e861f9e 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryTemplateTransformMappingsTests.java @@ -83,7 +83,7 @@ public void testTransformFields() throws Exception { .filter(Objects::nonNull) .findFirst(); - assertTrue(mapping.isPresent()); + assertTrue(mapping.isEmpty()); }); } }