From 13b71ee3edbcaa6ebec6177940b0ee8e854cda45 Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Fri, 25 Feb 2022 13:52:13 -0600 Subject: [PATCH 1/5] [Remove] deprecated getMapping API from IndicesClient Removes the deprecated types based get, getMapping, getAsync, and getMappingAsync methods from IndicesClient. It also removes extra nesting of mappings belong the deprecated type named object and removes the types based methods from the affected request classes. Signed-off-by: Nicholas Walter Knize --- .../org/opensearch/client/IndicesClient.java | 104 --------- .../client/IndicesRequestConverters.java | 16 -- .../client/AbstractResponseTestCase.java | 17 ++ .../opensearch/client/IndicesClientIT.java | 28 --- .../client/IndicesRequestConvertersTests.java | 47 ---- .../client/indices/GetIndexResponseTests.java | 126 ++++------ .../GetIndexTemplatesResponseTests.java | 11 +- .../indices/GetMappingsResponseTests.java | 82 ++----- .../index/mapper/size/SizeMappingIT.java | 19 +- .../admin/indices/create/CreateIndexIT.java | 20 +- .../action/admin/indices/get/GetIndexIT.java | 29 ++- .../documentation/IndicesDocumentationIT.java | 11 +- .../cluster/SimpleClusterStateIT.java | 13 +- .../coordination/RareClusterStateIT.java | 10 +- .../opensearch/gateway/MetadataNodesIT.java | 12 +- .../org/opensearch/index/HiddenIndexIT.java | 2 +- .../index/mapper/DynamicMappingIT.java | 7 +- .../mapper/MultiFieldsIntegrationIT.java | 10 +- .../indices/IndicesOptionsIntegrationIT.java | 24 +- .../indices/mapping/SimpleGetMappingsIT.java | 50 +--- .../mapping/UpdateMappingIntegrationIT.java | 18 +- .../admin/indices/get/GetIndexResponse.java | 218 +++--------------- .../indices/get/TransportGetIndexAction.java | 5 +- .../mapping/get/GetMappingsResponse.java | 116 ++++------ .../get/TransportGetMappingsAction.java | 4 +- .../master/info/ClusterInfoRequest.java | 20 +- .../info/ClusterInfoRequestBuilder.java | 12 - .../cluster/metadata/MappingMetadata.java | 33 ++- .../opensearch/cluster/metadata/Metadata.java | 51 +--- .../indices/get/GetIndexResponseTests.java | 35 +-- .../mapping/get/GetMappingsResponseTests.java | 87 ++----- .../cluster/metadata/MetadataTests.java | 92 ++------ .../mapper/FieldFilterMapperPluginTests.java | 22 +- 33 files changed, 332 insertions(+), 1019 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java index 00b07fc4881bd..66e28e429036a 100644 --- a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java +++ b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesClient.java @@ -550,60 +550,6 @@ public Cancellable getMappingAsync( ); } - /** - * Retrieves the mappings on an index or indices using the Get Mapping API. - * - * @param getMappingsRequest the request - * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized - * @return the response - * @throws IOException in case there is a problem sending the request or parsing back the response - * - * @deprecated This method uses old request and response objects which still refer to types, a deprecated - * feature. The method {@link #getMapping(GetMappingsRequest, RequestOptions)} should be used instead, which - * accepts a new request object. - */ - @Deprecated - public org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse getMapping( - org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest getMappingsRequest, - RequestOptions options - ) throws IOException { - return restHighLevelClient.performRequestAndParseEntity( - getMappingsRequest, - IndicesRequestConverters::getMappings, - options, - org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse::fromXContent, - emptySet() - ); - } - - /** - * Asynchronously retrieves the mappings on an index on indices using the Get Mapping API. - * - * @param getMappingsRequest the request - * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized - * @param listener the listener to be notified upon request completion - * - * @deprecated This method uses old request and response objects which still refer to types, a deprecated feature. - * The method {@link #getMapping(GetMappingsRequest, RequestOptions)} should be used instead, which accepts a new - * request object. - * @return cancellable that may be used to cancel the request - */ - @Deprecated - public Cancellable getMappingAsync( - org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest getMappingsRequest, - RequestOptions options, - ActionListener listener - ) { - return restHighLevelClient.performRequestAsyncAndParseEntity( - getMappingsRequest, - IndicesRequestConverters::getMappings, - options, - org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse::fromXContent, - listener, - emptySet() - ); - } - /** * Retrieves the field mappings on an index or indices using the Get Field Mapping API. * @@ -1008,56 +954,6 @@ public Cancellable getAsync(GetIndexRequest getIndexRequest, RequestOptions opti ); } - /** - * Retrieve information about one or more indexes - * - * @param getIndexRequest the request - * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized - * @return the response - * @throws IOException in case there is a problem sending the request or parsing back the response - * @deprecated This method uses an old request object which still refers to types, a deprecated feature. The method - * {@link #get(GetIndexRequest, RequestOptions)} should be used instead, which accepts a new request object. - */ - @Deprecated - public org.opensearch.action.admin.indices.get.GetIndexResponse get( - org.opensearch.action.admin.indices.get.GetIndexRequest getIndexRequest, - RequestOptions options - ) throws IOException { - return restHighLevelClient.performRequestAndParseEntity( - getIndexRequest, - IndicesRequestConverters::getIndex, - options, - org.opensearch.action.admin.indices.get.GetIndexResponse::fromXContent, - emptySet() - ); - } - - /** - * Retrieve information about one or more indexes - * - * @param getIndexRequest the request - * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized - * @param listener the listener to be notified upon request completion - * @deprecated This method uses an old request object which still refers to types, a deprecated feature. The method - * {@link #getAsync(GetIndexRequest, RequestOptions, ActionListener)} should be used instead, which accepts a new request object. - * @return cancellable that may be used to cancel the request - */ - @Deprecated - public Cancellable getAsync( - org.opensearch.action.admin.indices.get.GetIndexRequest getIndexRequest, - RequestOptions options, - ActionListener listener - ) { - return restHighLevelClient.performRequestAsyncAndParseEntity( - getIndexRequest, - IndicesRequestConverters::getIndex, - options, - org.opensearch.action.admin.indices.get.GetIndexResponse::fromXContent, - listener, - emptySet() - ); - } - /** * Force merge one or more indices using the Force Merge API. * diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java index 5078de05c6b89..0f55c722d09fb 100644 --- a/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/opensearch/client/IndicesRequestConverters.java @@ -240,22 +240,6 @@ static Request getMappings(GetMappingsRequest getMappingsRequest) { return request; } - @Deprecated - static Request getMappings(org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest getMappingsRequest) { - String[] indices = getMappingsRequest.indices() == null ? Strings.EMPTY_ARRAY : getMappingsRequest.indices(); - String[] types = getMappingsRequest.types() == null ? Strings.EMPTY_ARRAY : getMappingsRequest.types(); - - Request request = new Request(HttpGet.METHOD_NAME, RequestConverters.endpoint(indices, "_mapping", types)); - - RequestConverters.Params parameters = new RequestConverters.Params(); - parameters.withMasterTimeout(getMappingsRequest.masterNodeTimeout()); - parameters.withIndicesOptions(getMappingsRequest.indicesOptions()); - parameters.withLocal(getMappingsRequest.local()); - parameters.putParam(INCLUDE_TYPE_NAME_PARAMETER, "true"); - request.addParameters(parameters.asMap()); - return request; - } - static Request getFieldMapping(GetFieldMappingsRequest getFieldMappingsRequest) { String[] indices = getFieldMappingsRequest.indices() == null ? Strings.EMPTY_ARRAY : getFieldMappingsRequest.indices(); String[] fields = getFieldMappingsRequest.fields() == null ? Strings.EMPTY_ARRAY : getFieldMappingsRequest.fields(); diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/AbstractResponseTestCase.java b/client/rest-high-level/src/test/java/org/opensearch/client/AbstractResponseTestCase.java index 06ceee78e5566..95188ec0f8e96 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/AbstractResponseTestCase.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/AbstractResponseTestCase.java @@ -32,6 +32,7 @@ package org.opensearch.client; import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.NamedXContentRegistry; import org.opensearch.common.xcontent.ToXContent; @@ -42,6 +43,10 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Map; +import java.util.Set; /** * Base class for HLRC response parsing tests. @@ -99,4 +104,16 @@ protected ToXContent.Params getParams() { return ToXContent.EMPTY_PARAMS; } + protected static void assertMapEquals(ImmutableOpenMap expected, Map actual) { + Set expectedKeys = new HashSet<>(); + Iterator keysIt = expected.keysIt(); + while (keysIt.hasNext()) { + expectedKeys.add(keysIt.next()); + } + + assertEquals(expectedKeys, actual.keySet()); + for (String key : expectedKeys) { + assertEquals(expected.get(key), actual.get(key)); + } + } } diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java index 69ce518173042..0e50d55880e2e 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java @@ -128,7 +128,6 @@ import org.opensearch.rest.RestStatus; import org.opensearch.rest.action.admin.indices.RestCreateIndexAction; import org.opensearch.rest.action.admin.indices.RestGetIndexTemplateAction; -import org.opensearch.rest.action.admin.indices.RestGetIndicesAction; import org.opensearch.rest.action.admin.indices.RestPutIndexTemplateAction; import org.opensearch.rest.action.admin.indices.RestRolloverIndexAction; @@ -487,33 +486,6 @@ public void testGetIndex() throws IOException { assertEquals("integer", fieldMapping.get("type")); } - @SuppressWarnings("unchecked") - public void testGetIndexWithTypes() throws IOException { - String indexName = "get_index_test"; - Settings basicSettings = Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0).build(); - String mappings = "\"properties\":{\"field-1\":{\"type\":\"integer\"}}"; - createIndex(indexName, basicSettings, mappings); - - org.opensearch.action.admin.indices.get.GetIndexRequest getIndexRequest = - new org.opensearch.action.admin.indices.get.GetIndexRequest().indices(indexName).includeDefaults(false); - org.opensearch.action.admin.indices.get.GetIndexResponse getIndexResponse = execute( - getIndexRequest, - highLevelClient().indices()::get, - highLevelClient().indices()::getAsync, - expectWarningsOnce(RestGetIndicesAction.TYPES_DEPRECATION_MESSAGE) - ); - - // default settings should be null - assertNull(getIndexResponse.getSetting(indexName, "index.refresh_interval")); - assertEquals("1", getIndexResponse.getSetting(indexName, SETTING_NUMBER_OF_SHARDS)); - assertEquals("0", getIndexResponse.getSetting(indexName, SETTING_NUMBER_OF_REPLICAS)); - assertNotNull(getIndexResponse.getMappings().get(indexName)); - MappingMetadata mappingMetadata = getIndexResponse.getMappings().get(indexName).get("_doc"); - assertNotNull(mappingMetadata); - assertEquals("_doc", mappingMetadata.type()); - assertEquals("{\"properties\":{\"field-1\":{\"type\":\"integer\"}}}", mappingMetadata.source().string()); - } - @SuppressWarnings("unchecked") public void testGetIndexWithDefaults() throws IOException { String indexName = "get_index_test"; diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java index abb3da703fdaa..7fbcdc7d83133 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesRequestConvertersTests.java @@ -318,53 +318,6 @@ public void testGetMapping() { Assert.assertThat(HttpGet.METHOD_NAME, equalTo(request.getMethod())); } - public void testGetMappingWithTypes() { - org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest getMappingRequest = - new org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest(); - - String[] indices = Strings.EMPTY_ARRAY; - if (randomBoolean()) { - indices = RequestConvertersTests.randomIndicesNames(0, 5); - getMappingRequest.indices(indices); - } else if (randomBoolean()) { - getMappingRequest.indices((String[]) null); - } - - String type = null; - if (randomBoolean()) { - type = randomAlphaOfLengthBetween(3, 10); - getMappingRequest.types(type); - } else if (randomBoolean()) { - getMappingRequest.types((String[]) null); - } - - Map expectedParams = new HashMap<>(); - - RequestConvertersTests.setRandomIndicesOptions( - getMappingRequest::indicesOptions, - getMappingRequest::indicesOptions, - expectedParams - ); - RequestConvertersTests.setRandomMasterTimeout(getMappingRequest, expectedParams); - RequestConvertersTests.setRandomLocal(getMappingRequest::local, expectedParams); - expectedParams.put(INCLUDE_TYPE_NAME_PARAMETER, "true"); - - Request request = IndicesRequestConverters.getMappings(getMappingRequest); - StringJoiner endpoint = new StringJoiner("/", "/", ""); - String index = String.join(",", indices); - if (Strings.hasLength(index)) { - endpoint.add(index); - } - endpoint.add("_mapping"); - if (type != null) { - endpoint.add(type); - } - Assert.assertThat(endpoint.toString(), equalTo(request.getEndpoint())); - - Assert.assertThat(expectedParams, equalTo(request.getParameters())); - Assert.assertThat(HttpGet.METHOD_NAME, equalTo(request.getMethod())); - } - public void testGetFieldMapping() { GetFieldMappingsRequest getFieldMappingsRequest = new GetFieldMappingsRequest(); diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexResponseTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexResponseTests.java index 2141ce30dce64..37f4d95d5f4d0 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexResponseTests.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexResponseTests.java @@ -33,19 +33,17 @@ package org.opensearch.client.indices; import org.apache.lucene.util.CollectionUtil; +import org.opensearch.client.AbstractResponseTestCase; import org.opensearch.client.GetAliasesResponseTests; import org.opensearch.cluster.metadata.AliasMetadata; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.xcontent.ToXContent; -import org.opensearch.common.xcontent.ToXContent.Params; -import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.index.RandomCreateIndexGenerator; import org.opensearch.index.mapper.MapperService; -import org.opensearch.rest.BaseRestHandler; -import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; import java.util.ArrayList; @@ -57,40 +55,18 @@ import java.util.Map; import java.util.Objects; -import static org.opensearch.test.AbstractXContentTestCase.xContentTester; - -public class GetIndexResponseTests extends OpenSearchTestCase { - - // Because the client-side class does not have a toXContent method, we test xContent serialization by creating - // a random client object, converting it to a server object then serializing it to xContent, and finally - // parsing it back as a client object. We check equality between the original client object, and the parsed one. - public void testFromXContent() throws IOException { - xContentTester( - this::createParser, - GetIndexResponseTests::createTestInstance, - GetIndexResponseTests::toXContent, - GetIndexResponse::fromXContent - ).supportsUnknownFields(false) - .assertToXContentEquivalence(false) - .assertEqualsConsumer(GetIndexResponseTests::assertEqualInstances) - .test(); - } - - private static void assertEqualInstances(GetIndexResponse expected, GetIndexResponse actual) { - assertArrayEquals(expected.getIndices(), actual.getIndices()); - assertEquals(expected.getMappings(), actual.getMappings()); - assertEquals(expected.getSettings(), actual.getSettings()); - assertEquals(expected.getDefaultSettings(), actual.getDefaultSettings()); - assertEquals(expected.getAliases(), actual.getAliases()); - } +public class GetIndexResponseTests extends AbstractResponseTestCase< + org.opensearch.action.admin.indices.get.GetIndexResponse, + GetIndexResponse> { - private static GetIndexResponse createTestInstance() { + @Override + protected org.opensearch.action.admin.indices.get.GetIndexResponse createServerTestInstance(XContentType xContentType) { String[] indices = generateRandomStringArray(5, 5, false, false); - Map mappings = new HashMap<>(); - Map> aliases = new HashMap<>(); - Map settings = new HashMap<>(); - Map defaultSettings = new HashMap<>(); - Map dataStreams = new HashMap<>(); + ImmutableOpenMap.Builder mappings = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder> aliases = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder settings = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder defaultSettings = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder dataStreams = ImmutableOpenMap.builder(); IndexScopedSettings indexScopedSettings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS; boolean includeDefaults = randomBoolean(); for (String index : indices) { @@ -116,17 +92,36 @@ private static GetIndexResponse createTestInstance() { dataStreams.put(index, randomAlphaOfLength(5).toLowerCase(Locale.ROOT)); } } - return new GetIndexResponse(indices, mappings, aliases, settings, defaultSettings, dataStreams); + return new org.opensearch.action.admin.indices.get.GetIndexResponse( + indices, + mappings.build(), + aliases.build(), + settings.build(), + defaultSettings.build(), + dataStreams.build() + ); + } + + @Override + protected GetIndexResponse doParseToClientInstance(XContentParser parser) throws IOException { + return GetIndexResponse.fromXContent(parser); + } + + @Override + protected void assertInstances( + org.opensearch.action.admin.indices.get.GetIndexResponse serverTestInstance, + GetIndexResponse clientInstance + ) { + assertArrayEquals(serverTestInstance.getIndices(), clientInstance.getIndices()); + assertMapEquals(serverTestInstance.getMappings(), clientInstance.getMappings()); + assertMapEquals(serverTestInstance.getSettings(), clientInstance.getSettings()); + assertMapEquals(serverTestInstance.defaultSettings(), clientInstance.getDefaultSettings()); + assertMapEquals(serverTestInstance.getAliases(), clientInstance.getAliases()); } private static MappingMetadata createMappingsForIndex() { int typeCount = rarely() ? 0 : 1; - MappingMetadata mmd; - try { - mmd = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, Collections.emptyMap()); - } catch (IOException e) { - throw new RuntimeException(e); - } + MappingMetadata mmd = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, Collections.emptyMap()); for (int i = 0; i < typeCount; i++) { if (rarely() == false) { // rarely have no fields Map mappings = new HashMap<>(); @@ -135,12 +130,8 @@ private static MappingMetadata createMappingsForIndex() { mappings.put("field2-" + i, randomFieldMapping()); } - try { - String typeName = MapperService.SINGLE_MAPPING_NAME; - mmd = new MappingMetadata(typeName, mappings); - } catch (IOException e) { - fail("shouldn't have failed " + e); - } + String typeName = MapperService.SINGLE_MAPPING_NAME; + mmd = new MappingMetadata(typeName, mappings); } } return mmd; @@ -178,39 +169,4 @@ private static Map randomFieldMapping() { } return mappings; } - - private static void toXContent(GetIndexResponse response, XContentBuilder builder) throws IOException { - // first we need to repackage from GetIndexResponse to org.opensearch.action.admin.indices.get.GetIndexResponse - ImmutableOpenMap.Builder> allMappings = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder> aliases = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder settings = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder defaultSettings = ImmutableOpenMap.builder(); - - Map indexMappings = response.getMappings(); - for (String index : response.getIndices()) { - MappingMetadata mmd = indexMappings.get(index); - ImmutableOpenMap.Builder typedMappings = ImmutableOpenMap.builder(); - if (mmd != null) { - typedMappings.put(MapperService.SINGLE_MAPPING_NAME, mmd); - } - allMappings.put(index, typedMappings.build()); - aliases.put(index, response.getAliases().get(index)); - settings.put(index, response.getSettings().get(index)); - defaultSettings.put(index, response.getDefaultSettings().get(index)); - } - - org.opensearch.action.admin.indices.get.GetIndexResponse serverResponse = - new org.opensearch.action.admin.indices.get.GetIndexResponse( - response.getIndices(), - allMappings.build(), - aliases.build(), - settings.build(), - defaultSettings.build(), - ImmutableOpenMap.builder().build() - ); - - // then we can call its toXContent method, forcing no output of types - Params params = new ToXContent.MapParams(Collections.singletonMap(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "false")); - serverResponse.toXContent(builder, params); - } } diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexTemplatesResponseTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexTemplatesResponseTests.java index 4049fcb41df99..b28da63e4344a 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexTemplatesResponseTests.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetIndexTemplatesResponseTests.java @@ -50,7 +50,6 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; -import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; @@ -196,13 +195,9 @@ static GetIndexTemplatesResponse createTestInstance() { templateBuilder.version(between(0, 100)); } if (randomBoolean()) { - try { - Map map = XContentHelper.convertToMap(new BytesArray(mappingString), true, XContentType.JSON).v2(); - MappingMetadata mapping = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, map); - templateBuilder.mapping(mapping); - } catch (IOException ex) { - throw new UncheckedIOException(ex); - } + Map map = XContentHelper.convertToMap(new BytesArray(mappingString), true, XContentType.JSON).v2(); + MappingMetadata mapping = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, map); + templateBuilder.mapping(mapping); } templates.add(templateBuilder.build()); } diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetMappingsResponseTests.java b/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetMappingsResponseTests.java index 817bce359b7d7..cb62b116de020 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetMappingsResponseTests.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/indices/GetMappingsResponseTests.java @@ -32,70 +32,54 @@ package org.opensearch.client.indices; +import org.opensearch.client.AbstractResponseTestCase; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.collect.ImmutableOpenMap; -import org.opensearch.common.xcontent.ToXContent; -import org.opensearch.common.xcontent.ToXContent.Params; -import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.index.mapper.MapperService; -import org.opensearch.rest.BaseRestHandler; -import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.function.Predicate; -import static org.opensearch.client.indices.GetMappingsResponse.MAPPINGS; -import static org.opensearch.test.AbstractXContentTestCase.xContentTester; +public class GetMappingsResponseTests extends AbstractResponseTestCase< + org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse, + GetMappingsResponse> { -public class GetMappingsResponseTests extends OpenSearchTestCase { - - // Because the client-side class does not have a toXContent method, we test xContent serialization by creating - // a random client object, converting it to a server object then serializing it to xContent, and finally - // parsing it back as a client object. We check equality between the original client object, and the parsed one. - public void testFromXContent() throws IOException { - xContentTester( - this::createParser, - GetMappingsResponseTests::createTestInstance, - GetMappingsResponseTests::toXContent, - GetMappingsResponse::fromXContent - ).supportsUnknownFields(true) - .assertEqualsConsumer(GetMappingsResponseTests::assertEqualInstances) - .randomFieldsExcludeFilter(randomFieldsExcludeFilter()) - .test(); - } - - private static GetMappingsResponse createTestInstance() { - Map mappings = Collections.singletonMap("index-" + randomAlphaOfLength(5), randomMappingMetadata()); - return new GetMappingsResponse(mappings); + @Override + protected org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse createServerTestInstance(XContentType xContentType) { + ImmutableOpenMap.Builder mappings = ImmutableOpenMap.builder(); + int numberOfIndexes = randomIntBetween(1, 5); + for (int i = 0; i < numberOfIndexes; i++) { + mappings.put("index-" + randomAlphaOfLength(5), randomMappingMetadata()); + } + return new org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse(mappings.build()); } - private static void assertEqualInstances(GetMappingsResponse expected, GetMappingsResponse actual) { - assertEquals(expected.mappings(), actual.mappings()); + @Override + protected GetMappingsResponse doParseToClientInstance(XContentParser parser) throws IOException { + return GetMappingsResponse.fromXContent(parser); } - private Predicate randomFieldsExcludeFilter() { - return field -> !field.equals(MAPPINGS.getPreferredName()); + @Override + protected void assertInstances( + org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse serverTestInstance, + GetMappingsResponse clientInstance + ) { + assertMapEquals(serverTestInstance.getMappings(), clientInstance.mappings()); } public static MappingMetadata randomMappingMetadata() { Map mappings = new HashMap<>(); - if (frequently()) { // rarely have no fields mappings.put("field1", randomFieldMapping()); if (randomBoolean()) { mappings.put("field2", randomFieldMapping()); } } - - try { - return new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, mappings); - } catch (IOException e) { - throw new RuntimeException(e); - } + return new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, mappings); } private static Map randomFieldMapping() { @@ -110,22 +94,4 @@ private static Map randomFieldMapping() { } return mappings; } - - private static void toXContent(GetMappingsResponse response, XContentBuilder builder) throws IOException { - Params params = new ToXContent.MapParams(Collections.singletonMap(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "false")); - ImmutableOpenMap.Builder> allMappings = ImmutableOpenMap.builder(); - - for (Map.Entry indexEntry : response.mappings().entrySet()) { - ImmutableOpenMap.Builder mappings = ImmutableOpenMap.builder(); - mappings.put(MapperService.SINGLE_MAPPING_NAME, indexEntry.getValue()); - allMappings.put(indexEntry.getKey(), mappings.build()); - } - - org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse serverResponse = - new org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse(allMappings.build()); - - builder.startObject(); - serverResponse.toXContent(builder, params); - builder.endObject(); - } } diff --git a/plugins/mapper-size/src/internalClusterTest/java/org/opensearch/index/mapper/size/SizeMappingIT.java b/plugins/mapper-size/src/internalClusterTest/java/org/opensearch/index/mapper/size/SizeMappingIT.java index c4f64ccc53a50..dc7a6e9467e70 100644 --- a/plugins/mapper-size/src/internalClusterTest/java/org/opensearch/index/mapper/size/SizeMappingIT.java +++ b/plugins/mapper-size/src/internalClusterTest/java/org/opensearch/index/mapper/size/SizeMappingIT.java @@ -68,7 +68,7 @@ public void testThatUpdatingMappingShouldNotRemoveSizeMappingConfiguration() thr assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder)); // check mapping again - assertSizeMappingEnabled(index, type, true); + assertSizeMappingEnabled(index, true); // update some field in the mapping XContentBuilder updateMappingBuilder = jsonBuilder().startObject() @@ -87,7 +87,7 @@ public void testThatUpdatingMappingShouldNotRemoveSizeMappingConfiguration() thr assertAcked(putMappingResponse); // make sure size field is still in mapping - assertSizeMappingEnabled(index, type, true); + assertSizeMappingEnabled(index, true); } public void testThatSizeCanBeSwitchedOnAndOff() throws Exception { @@ -98,7 +98,7 @@ public void testThatSizeCanBeSwitchedOnAndOff() throws Exception { assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder)); // check mapping again - assertSizeMappingEnabled(index, type, true); + assertSizeMappingEnabled(index, true); // update some field in the mapping XContentBuilder updateMappingBuilder = jsonBuilder().startObject() @@ -115,18 +115,17 @@ public void testThatSizeCanBeSwitchedOnAndOff() throws Exception { assertAcked(putMappingResponse); // make sure size field is still in mapping - assertSizeMappingEnabled(index, type, false); + assertSizeMappingEnabled(index, false); } - private void assertSizeMappingEnabled(String index, String type, boolean enabled) throws IOException { + private void assertSizeMappingEnabled(String index, boolean enabled) throws IOException { String errMsg = String.format( Locale.ROOT, - "Expected size field mapping to be " + (enabled ? "enabled" : "disabled") + " for %s/%s", - index, - type + "Expected size field mapping to be " + (enabled ? "enabled" : "disabled") + " for %s", + index ); - GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes(type).get(); - Map mappingSource = getMappingsResponse.getMappings().get(index).get(type).getSourceAsMap(); + GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).get(); + Map mappingSource = getMappingsResponse.getMappings().get(index).getSourceAsMap(); assertThat(errMsg, mappingSource, hasKey("_size")); String sizeAsString = mappingSource.get("_size").toString(); assertThat(sizeAsString, is(notNullValue())); diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java index 40c0c879ae0af..311767d82ac6c 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java @@ -148,12 +148,9 @@ public void testNonNestedMappings() throws Exception { GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); - ImmutableOpenMap mappings = response.mappings().get("test"); + MappingMetadata mappings = response.mappings().get("test"); assertNotNull(mappings); - - MappingMetadata metadata = mappings.get("_doc"); - assertNotNull(metadata); - assertFalse(metadata.sourceAsMap().isEmpty()); + assertFalse(mappings.sourceAsMap().isEmpty()); } public void testEmptyNestedMappings() throws Exception { @@ -161,12 +158,10 @@ public void testEmptyNestedMappings() throws Exception { GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); - ImmutableOpenMap mappings = response.mappings().get("test"); + MappingMetadata mappings = response.mappings().get("test"); assertNotNull(mappings); - MappingMetadata metadata = mappings.get("_doc"); - assertNotNull(metadata); - assertTrue(metadata.sourceAsMap().isEmpty()); + assertTrue(mappings.sourceAsMap().isEmpty()); } public void testMappingParamAndNestedMismatch() throws Exception { @@ -190,12 +185,9 @@ public void testEmptyMappings() throws Exception { GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); - ImmutableOpenMap mappings = response.mappings().get("test"); + MappingMetadata mappings = response.mappings().get("test"); assertNotNull(mappings); - - MappingMetadata metadata = mappings.get("_doc"); - assertNotNull(metadata); - assertTrue(metadata.sourceAsMap().isEmpty()); + assertTrue(mappings.sourceAsMap().isEmpty()); } public void testInvalidShardCountSettings() throws Exception { diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/get/GetIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/get/GetIndexIT.java index ad6c9ecfb5663..bbe8b616ad87e 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/get/GetIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/get/GetIndexIT.java @@ -35,6 +35,7 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.opensearch.action.admin.indices.alias.Alias; import org.opensearch.action.admin.indices.get.GetIndexRequest.Feature; +import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.metadata.AliasMetadata; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.collect.ImmutableOpenMap; @@ -94,6 +95,19 @@ public void testSimpleUnknownIndex() { } } + public void testUnknownIndexWithAllowNoIndices() { + GetIndexResponse response = client().admin() + .indices() + .prepareGetIndex() + .addIndices("missing_idx") + .setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN) + .get(); + assertThat(response.indices(), notNullValue()); + assertThat(response.indices().length, equalTo(0)); + assertThat(response.mappings(), notNullValue()); + assertThat(response.mappings().size(), equalTo(0)); + } + public void testEmpty() { GetIndexResponse response = client().admin().indices().prepareGetIndex().addIndices("empty_idx").get(); String[] indices = response.indices(); @@ -263,24 +277,19 @@ private void assertNonEmptySettings(GetIndexResponse response, String indexName) } private void assertMappings(GetIndexResponse response, String indexName) { - ImmutableOpenMap> mappings = response.mappings(); + ImmutableOpenMap mappings = response.mappings(); assertThat(mappings, notNullValue()); assertThat(mappings.size(), equalTo(1)); - ImmutableOpenMap indexMappings = mappings.get(indexName); + MappingMetadata indexMappings = mappings.get(indexName); assertThat(indexMappings, notNullValue()); - assertThat(indexMappings.size(), equalTo(1)); - MappingMetadata mapping = indexMappings.get("type1"); - assertThat(mapping, notNullValue()); - assertThat(mapping.type(), equalTo("type1")); } private void assertEmptyOrOnlyDefaultMappings(GetIndexResponse response, String indexName) { - ImmutableOpenMap> mappings = response.mappings(); + ImmutableOpenMap mappings = response.mappings(); assertThat(mappings, notNullValue()); assertThat(mappings.size(), equalTo(1)); - ImmutableOpenMap indexMappings = mappings.get(indexName); - assertThat(indexMappings, notNullValue()); - assertThat(indexMappings.size(), equalTo(0)); + MappingMetadata indexMappings = mappings.get(indexName); + assertEquals(indexMappings, MappingMetadata.EMPTY_MAPPINGS); } private void assertAliases(GetIndexResponse response, String indexName) { diff --git a/server/src/internalClusterTest/java/org/opensearch/client/documentation/IndicesDocumentationIT.java b/server/src/internalClusterTest/java/org/opensearch/client/documentation/IndicesDocumentationIT.java index 6108e8ee8efe3..bea1dc8b8dcc2 100644 --- a/server/src/internalClusterTest/java/org/opensearch/client/documentation/IndicesDocumentationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/client/documentation/IndicesDocumentationIT.java @@ -35,12 +35,10 @@ import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse; import org.opensearch.client.Client; import org.opensearch.cluster.metadata.MappingMetadata; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.xcontent.XContentType; import org.opensearch.test.OpenSearchIntegTestCase; import static java.util.Collections.singletonMap; -import static org.hamcrest.Matchers.instanceOf; /** * This class is used to generate the Java indices administration documentation. @@ -73,8 +71,6 @@ public void testPutMappingDocumentation() throws Exception { // end::index-with-mapping GetMappingsResponse getMappingsResponse = client.admin().indices().prepareGetMappings("twitter").get(); assertEquals(1, getMappingsResponse.getMappings().size()); - ImmutableOpenMap indexMapping = getMappingsResponse.getMappings().get("twitter"); - assertThat(indexMapping.get("_doc"), instanceOf(MappingMetadata.class)); // we need to delete in order to create a fresh new index with another type client.admin().indices().prepareDelete("twitter").get(); @@ -108,11 +104,8 @@ public void testPutMappingDocumentation() throws Exception { // end::putMapping-request-source getMappingsResponse = client.admin().indices().prepareGetMappings("twitter").get(); assertEquals(1, getMappingsResponse.getMappings().size()); - indexMapping = getMappingsResponse.getMappings().get("twitter"); - assertEquals( - singletonMap("properties", singletonMap("name", singletonMap("type", "text"))), - indexMapping.get("_doc").getSourceAsMap() - ); + MappingMetadata indexMapping = getMappingsResponse.getMappings().get("twitter"); + assertEquals(singletonMap("properties", singletonMap("name", singletonMap("type", "text"))), indexMapping.getSourceAsMap()); } } diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/SimpleClusterStateIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/SimpleClusterStateIT.java index 931a67655a92f..93a903e0b5e0c 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/SimpleClusterStateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/SimpleClusterStateIT.java @@ -292,24 +292,15 @@ public void testLargeClusterStatePublishing() throws Exception { .get() ); ensureGreen(); // wait for green state, so its both green, and there are no more pending events - MappingMetadata masterMappingMetadata = client().admin() - .indices() - .prepareGetMappings("test") - .setTypes("type") - .get() - .getMappings() - .get("test") - .get("type"); + MappingMetadata masterMappingMetadata = client().admin().indices().prepareGetMappings("test").get().getMappings().get("test"); for (Client client : clients()) { MappingMetadata mappingMetadata = client.admin() .indices() .prepareGetMappings("test") - .setTypes("type") .setLocal(true) .get() .getMappings() - .get("test") - .get("type"); + .get("test"); assertThat(mappingMetadata.source().string(), equalTo(masterMappingMetadata.source().string())); assertThat(mappingMetadata, equalTo(masterMappingMetadata)); } diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/coordination/RareClusterStateIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/coordination/RareClusterStateIT.java index 63d674386e94e..773660f3fb670 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/coordination/RareClusterStateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/coordination/RareClusterStateIT.java @@ -53,7 +53,6 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.allocation.AllocationService; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.discovery.Discovery; @@ -269,14 +268,7 @@ public void testDelayedMappingPropagationOnPrimary() throws Exception { // ...and wait for mappings to be available on master assertBusy(() -> { - ImmutableOpenMap indexMappings = client().admin() - .indices() - .prepareGetMappings("index") - .get() - .getMappings() - .get("index"); - assertNotNull(indexMappings); - MappingMetadata typeMappings = indexMappings.get("type"); + MappingMetadata typeMappings = client().admin().indices().prepareGetMappings("index").get().getMappings().get("index"); assertNotNull(typeMappings); Object properties; try { diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/MetadataNodesIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/MetadataNodesIT.java index c6f1996bd7eff..7e16a4ad8387a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/MetadataNodesIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/MetadataNodesIT.java @@ -147,11 +147,9 @@ public void testMetaWrittenWhenIndexIsClosedAndMetaUpdated() throws Exception { ) .get(); - GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes("_doc").get(); + GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).get(); assertNotNull( - ((Map) (getMappingsResponse.getMappings().get(index).get("_doc").getSourceAsMap().get("properties"))).get( - "integer_field" - ) + ((Map) (getMappingsResponse.getMappings().get(index).getSourceAsMap().get("properties"))).get("integer_field") ); // make sure it was also written on red node although index is closed @@ -187,11 +185,9 @@ public void testMetaWrittenWhenIndexIsClosedAndMetaUpdated() throws Exception { ) .get(); - getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes("_doc").get(); + getMappingsResponse = client().admin().indices().prepareGetMappings(index).get(); assertNotNull( - ((Map) (getMappingsResponse.getMappings().get(index).get("_doc").getSourceAsMap().get("properties"))).get( - "float_field" - ) + ((Map) (getMappingsResponse.getMappings().get(index).getSourceAsMap().get("properties"))).get("float_field") ); // make sure it was also written on red node although index is closed diff --git a/server/src/internalClusterTest/java/org/opensearch/index/HiddenIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/index/HiddenIndexIT.java index d51e16bbc7734..54fbc8cecb967 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/HiddenIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/HiddenIndexIT.java @@ -160,7 +160,7 @@ public void testGlobalTemplatesDoNotApply() { GetMappingsResponse mappingsResponse = client().admin().indices().prepareGetMappings("a_hidden_index").get(); assertThat(mappingsResponse.mappings().size(), is(1)); - MappingMetadata mappingMetadata = mappingsResponse.mappings().get("a_hidden_index").get("_doc"); + MappingMetadata mappingMetadata = mappingsResponse.mappings().get("a_hidden_index"); assertNotNull(mappingMetadata); Map propertiesMap = (Map) mappingMetadata.getSourceAsMap().get("properties"); assertNotNull(propertiesMap); diff --git a/server/src/internalClusterTest/java/org/opensearch/index/mapper/DynamicMappingIT.java b/server/src/internalClusterTest/java/org/opensearch/index/mapper/DynamicMappingIT.java index 11f3aeb852141..d5924155e2ec7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/mapper/DynamicMappingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/mapper/DynamicMappingIT.java @@ -38,7 +38,6 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.metadata.MappingMetadata; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; @@ -94,11 +93,9 @@ public void testConflictingDynamicMappingsBulk() { } private static void assertMappingsHaveField(GetMappingsResponse mappings, String index, String field) throws IOException { - ImmutableOpenMap indexMappings = mappings.getMappings().get("index"); + MappingMetadata indexMappings = mappings.getMappings().get("index"); assertNotNull(indexMappings); - MappingMetadata typeMappings = indexMappings.get(MapperService.SINGLE_MAPPING_NAME); - assertNotNull(typeMappings); - Map typeMappingsMap = typeMappings.getSourceAsMap(); + Map typeMappingsMap = indexMappings.getSourceAsMap(); Map properties = (Map) typeMappingsMap.get("properties"); assertTrue("Could not find [" + field + "] in " + typeMappingsMap.toString(), properties.containsKey(field)); } diff --git a/server/src/internalClusterTest/java/org/opensearch/index/mapper/MultiFieldsIntegrationIT.java b/server/src/internalClusterTest/java/org/opensearch/index/mapper/MultiFieldsIntegrationIT.java index 2ea27b0219ad9..e0a20b6b2b371 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/mapper/MultiFieldsIntegrationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/mapper/MultiFieldsIntegrationIT.java @@ -61,7 +61,7 @@ public void testMultiFields() throws Exception { assertAcked(client().admin().indices().prepareCreate("my-index").addMapping("my-type", createTypeSource())); GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("my-index").get(); - MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index").get("my-type"); + MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index"); assertThat(mappingMetadata, not(nullValue())); Map mappingSource = mappingMetadata.sourceAsMap(); Map titleFields = ((Map) XContentMapValues.extractValue("properties.title.fields", mappingSource)); @@ -79,7 +79,7 @@ public void testMultiFields() throws Exception { assertAcked(client().admin().indices().preparePutMapping("my-index").setType("my-type").setSource(createPutMappingSource())); getMappingsResponse = client().admin().indices().prepareGetMappings("my-index").get(); - mappingMetadata = getMappingsResponse.mappings().get("my-index").get("my-type"); + mappingMetadata = getMappingsResponse.mappings().get("my-index"); assertThat(mappingMetadata, not(nullValue())); mappingSource = mappingMetadata.sourceAsMap(); assertThat(((Map) XContentMapValues.extractValue("properties.title", mappingSource)).size(), equalTo(2)); @@ -101,7 +101,7 @@ public void testGeoPointMultiField() throws Exception { assertAcked(client().admin().indices().prepareCreate("my-index").addMapping("my-type", createMappingSource("geo_point"))); GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("my-index").get(); - MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index").get("my-type"); + MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index"); assertThat(mappingMetadata, not(nullValue())); Map mappingSource = mappingMetadata.sourceAsMap(); Map aField = ((Map) XContentMapValues.extractValue("properties.a", mappingSource)); @@ -130,7 +130,7 @@ public void testCompletionMultiField() throws Exception { assertAcked(client().admin().indices().prepareCreate("my-index").addMapping("my-type", createMappingSource("completion"))); GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("my-index").get(); - MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index").get("my-type"); + MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index"); assertThat(mappingMetadata, not(nullValue())); Map mappingSource = mappingMetadata.sourceAsMap(); Map aField = ((Map) XContentMapValues.extractValue("properties.a", mappingSource)); @@ -152,7 +152,7 @@ public void testIpMultiField() throws Exception { assertAcked(client().admin().indices().prepareCreate("my-index").addMapping("my-type", createMappingSource("ip"))); GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("my-index").get(); - MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index").get("my-type"); + MappingMetadata mappingMetadata = getMappingsResponse.mappings().get("my-index"); assertThat(mappingMetadata, not(nullValue())); Map mappingSource = mappingMetadata.sourceAsMap(); Map aField = ((Map) XContentMapValues.extractValue("properties.a", mappingSource)); diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/IndicesOptionsIntegrationIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/IndicesOptionsIntegrationIT.java index 3c08f0b3e6fa8..8c54b09626d78 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/IndicesOptionsIntegrationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/IndicesOptionsIntegrationIT.java @@ -603,26 +603,26 @@ public void testPutMapping() throws Exception { } verify(client().admin().indices().preparePutMapping("foo").setType("type").setSource("field", "type=text"), false); - assertThat(client().admin().indices().prepareGetMappings("foo").get().mappings().get("foo").get("type"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("foo").get().mappings().get("foo"), notNullValue()); verify(client().admin().indices().preparePutMapping("b*").setType("type").setSource("field", "type=text"), false); - assertThat(client().admin().indices().prepareGetMappings("bar").get().mappings().get("bar").get("type"), notNullValue()); - assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz").get("type"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("bar").get().mappings().get("bar"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz"), notNullValue()); verify(client().admin().indices().preparePutMapping("_all").setType("type").setSource("field", "type=text"), false); - assertThat(client().admin().indices().prepareGetMappings("foo").get().mappings().get("foo").get("type"), notNullValue()); - assertThat(client().admin().indices().prepareGetMappings("foobar").get().mappings().get("foobar").get("type"), notNullValue()); - assertThat(client().admin().indices().prepareGetMappings("bar").get().mappings().get("bar").get("type"), notNullValue()); - assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz").get("type"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("foo").get().mappings().get("foo"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("foobar").get().mappings().get("foobar"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("bar").get().mappings().get("bar"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz"), notNullValue()); verify(client().admin().indices().preparePutMapping().setType("type").setSource("field", "type=text"), false); - assertThat(client().admin().indices().prepareGetMappings("foo").get().mappings().get("foo").get("type"), notNullValue()); - assertThat(client().admin().indices().prepareGetMappings("foobar").get().mappings().get("foobar").get("type"), notNullValue()); - assertThat(client().admin().indices().prepareGetMappings("bar").get().mappings().get("bar").get("type"), notNullValue()); - assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz").get("type"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("foo").get().mappings().get("foo"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("foobar").get().mappings().get("foobar"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("bar").get().mappings().get("bar"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz"), notNullValue()); verify(client().admin().indices().preparePutMapping("c*").setType("type").setSource("field", "type=text"), true); assertAcked(client().admin().indices().prepareClose("barbaz").get()); verify(client().admin().indices().preparePutMapping("barbaz").setType("type").setSource("field", "type=text"), false); - assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz").get("type"), notNullValue()); + assertThat(client().admin().indices().prepareGetMappings("barbaz").get().mappings().get("barbaz"), notNullValue()); } public static final class TestPlugin extends Plugin { diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/mapping/SimpleGetMappingsIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/mapping/SimpleGetMappingsIT.java index cfd4a830e5c37..aac12522afa2f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/mapping/SimpleGetMappingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/mapping/SimpleGetMappingsIT.java @@ -34,6 +34,7 @@ import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse; +import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.Priority; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.plugins.Plugin; @@ -66,7 +67,7 @@ public void testGetMappingsWhereThereAreNone() { createIndex("index"); GetMappingsResponse response = client().admin().indices().prepareGetMappings().execute().actionGet(); assertThat(response.mappings().containsKey("index"), equalTo(true)); - assertThat(response.mappings().get("index").size(), equalTo(0)); + assertEquals(MappingMetadata.EMPTY_MAPPINGS, response.mappings().get("index")); } private XContentBuilder getMappingForType(String type) throws IOException { @@ -97,50 +98,19 @@ public void testSimpleGetMappings() throws Exception { // Get all mappings GetMappingsResponse response = client().admin().indices().prepareGetMappings().execute().actionGet(); assertThat(response.mappings().size(), equalTo(2)); - assertThat(response.mappings().get("indexa").size(), equalTo(1)); - assertThat(response.mappings().get("indexa").get("typeA"), notNullValue()); - assertThat(response.mappings().get("indexb").size(), equalTo(1)); - assertThat(response.mappings().get("indexb").get("typeA"), notNullValue()); + assertThat(response.mappings().get("indexa"), notNullValue()); + assertThat(response.mappings().get("indexb"), notNullValue()); // Get all mappings, via wildcard support - response = client().admin().indices().prepareGetMappings("*").setTypes("*").execute().actionGet(); + response = client().admin().indices().prepareGetMappings("*").execute().actionGet(); assertThat(response.mappings().size(), equalTo(2)); - assertThat(response.mappings().get("indexa").size(), equalTo(1)); - assertThat(response.mappings().get("indexa").get("typeA"), notNullValue()); - assertThat(response.mappings().get("indexb").size(), equalTo(1)); - assertThat(response.mappings().get("indexb").get("typeA"), notNullValue()); + assertThat(response.mappings().get("indexa"), notNullValue()); + assertThat(response.mappings().get("indexb"), notNullValue()); - // Get all typeA mappings in all indices - response = client().admin().indices().prepareGetMappings("*").setTypes("typeA").execute().actionGet(); - assertThat(response.mappings().size(), equalTo(2)); - assertThat(response.mappings().get("indexa").size(), equalTo(1)); - assertThat(response.mappings().get("indexa").get("typeA"), notNullValue()); - assertThat(response.mappings().get("indexb").size(), equalTo(1)); - assertThat(response.mappings().get("indexb").get("typeA"), notNullValue()); - - // Get all mappings in indexa + // Get mappings in indexa response = client().admin().indices().prepareGetMappings("indexa").execute().actionGet(); assertThat(response.mappings().size(), equalTo(1)); - assertThat(response.mappings().get("indexa").size(), equalTo(1)); - assertThat(response.mappings().get("indexa").get("typeA"), notNullValue()); - - // Get all mappings beginning with A* in indexa - response = client().admin().indices().prepareGetMappings("indexa").setTypes("*A").execute().actionGet(); - assertThat(response.mappings().size(), equalTo(1)); - assertThat(response.mappings().get("indexa").size(), equalTo(1)); - assertThat(response.mappings().get("indexa").get("typeA"), notNullValue()); - - // Get all mappings beginning with B* in all indices - response = client().admin().indices().prepareGetMappings().setTypes("B*").execute().actionGet(); - assertThat(response.mappings().size(), equalTo(0)); - - // Get all mappings beginning with B* and A* in all indices - response = client().admin().indices().prepareGetMappings().setTypes("B*", "*A").execute().actionGet(); - assertThat(response.mappings().size(), equalTo(2)); - assertThat(response.mappings().get("indexa").size(), equalTo(1)); - assertThat(response.mappings().get("indexa").get("typeA"), notNullValue()); - assertThat(response.mappings().get("indexb").size(), equalTo(1)); - assertThat(response.mappings().get("indexb").get("typeA"), notNullValue()); + assertThat(response.mappings().get("indexa"), notNullValue()); } public void testGetMappingsWithBlocks() throws IOException { @@ -152,7 +122,7 @@ public void testGetMappingsWithBlocks() throws IOException { enableIndexBlock("test", block); GetMappingsResponse response = client().admin().indices().prepareGetMappings().execute().actionGet(); assertThat(response.mappings().size(), equalTo(1)); - assertThat(response.mappings().get("test").size(), equalTo(1)); + assertNotNull(response.mappings().get("test")); } finally { disableIndexBlock("test", block); } diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/mapping/UpdateMappingIntegrationIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/mapping/UpdateMappingIntegrationIT.java index 896f2e75aee3c..889e71ecb7e6f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/mapping/UpdateMappingIntegrationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/mapping/UpdateMappingIntegrationIT.java @@ -41,7 +41,6 @@ import org.opensearch.cluster.action.index.MappingUpdatedAction; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.Priority; -import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.xcontent.XContentType; @@ -163,7 +162,7 @@ public void testUpdateMappingWithoutType() { GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").execute().actionGet(); assertThat( - getMappingsResponse.mappings().get("test").get("_doc").source().toString(), + getMappingsResponse.mappings().get("test").source().toString(), equalTo("{\"_doc\":{\"properties\":{\"body\":{\"type\":\"text\"},\"date\":{\"type\":\"integer\"}}}}") ); } @@ -189,7 +188,7 @@ public void testUpdateMappingWithoutTypeMultiObjects() { GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").execute().actionGet(); assertThat( - getMappingsResponse.mappings().get("test").get("_doc").source().toString(), + getMappingsResponse.mappings().get("test").source().toString(), equalTo("{\"_doc\":{\"properties\":{\"date\":{\"type\":\"integer\"}}}}") ); } @@ -313,10 +312,9 @@ public void testUpdateMappingConcurrently() throws Throwable { assertThat(response.isAcknowledged(), equalTo(true)); GetMappingsResponse getMappingResponse = client2.admin().indices().prepareGetMappings(indexName).get(); - ImmutableOpenMap mappings = getMappingResponse.getMappings().get(indexName); - assertThat(mappings.containsKey(typeName), equalTo(true)); + MappingMetadata mappings = getMappingResponse.getMappings().get(indexName); assertThat( - ((Map) mappings.get(typeName).getSourceAsMap().get("properties")).keySet(), + ((Map) mappings.getSourceAsMap().get("properties")).keySet(), Matchers.hasItem(fieldName) ); } @@ -399,12 +397,10 @@ private void assertConcreteMappingsOnAll(final String index, final String... fie */ private void assertMappingOnMaster(final String index, final String... fieldNames) { GetMappingsResponse response = client().admin().indices().prepareGetMappings(index).get(); - ImmutableOpenMap mappings = response.getMappings().get(index); + MappingMetadata mappings = response.getMappings().get(index); assertThat(mappings, notNullValue()); - MappingMetadata mappingMetadata = mappings.get(MapperService.SINGLE_MAPPING_NAME); - assertThat(mappingMetadata, notNullValue()); + Map mappingSource = mappings.getSourceAsMap(); - Map mappingSource = mappingMetadata.getSourceAsMap(); assertFalse(mappingSource.isEmpty()); assertTrue(mappingSource.containsKey("properties")); @@ -414,7 +410,7 @@ private void assertMappingOnMaster(final String index, final String... fieldName fieldName = fieldName.replace(".", ".properties."); } assertThat( - "field " + fieldName + " doesn't exists in mapping " + mappingMetadata.source().string(), + "field " + fieldName + " doesn't exists in mapping " + mappings.source().string(), XContentMapValues.extractValue(fieldName, mappingProperties), notNullValue() ); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java index 7efe88e9bbc83..730ba018c3471 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java @@ -33,8 +33,8 @@ package org.opensearch.action.admin.indices.get; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; -import org.apache.lucene.util.CollectionUtil; import org.opensearch.LegacyESVersion; +import org.opensearch.Version; import org.opensearch.action.ActionResponse; import org.opensearch.cluster.metadata.AliasMetadata; import org.opensearch.cluster.metadata.MappingMetadata; @@ -45,27 +45,21 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.ToXContentObject; import org.opensearch.common.xcontent.XContentBuilder; -import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.common.xcontent.XContentParser.Token; +import org.opensearch.index.mapper.MapperService; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Objects; -import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.opensearch.rest.BaseRestHandler.DEFAULT_INCLUDE_TYPE_NAME_POLICY; -import static org.opensearch.rest.BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER; - /** * A response for a get index action. */ public class GetIndexResponse extends ActionResponse implements ToXContentObject { - private ImmutableOpenMap> mappings = ImmutableOpenMap.of(); + private ImmutableOpenMap mappings = ImmutableOpenMap.of(); private ImmutableOpenMap> aliases = ImmutableOpenMap.of(); private ImmutableOpenMap settings = ImmutableOpenMap.of(); private ImmutableOpenMap defaultSettings = ImmutableOpenMap.of(); @@ -74,7 +68,7 @@ public class GetIndexResponse extends ActionResponse implements ToXContentObject public GetIndexResponse( String[] indices, - ImmutableOpenMap> mappings, + ImmutableOpenMap mappings, ImmutableOpenMap> aliases, ImmutableOpenMap settings, ImmutableOpenMap defaultSettings, @@ -105,15 +99,23 @@ public GetIndexResponse( this.indices = in.readStringArray(); int mappingsSize = in.readVInt(); - ImmutableOpenMap.Builder> mappingsMapBuilder = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder mappingsMapBuilder = ImmutableOpenMap.builder(); for (int i = 0; i < mappingsSize; i++) { - String key = in.readString(); - int valueSize = in.readVInt(); - ImmutableOpenMap.Builder mappingEntryBuilder = ImmutableOpenMap.builder(); - for (int j = 0; j < valueSize; j++) { - mappingEntryBuilder.put(in.readString(), new MappingMetadata(in)); + String index = in.readString(); + if (in.getVersion().before(Version.V_2_0_0)) { + int numMappings = in.readVInt(); + assert numMappings == 0 || numMappings == 1 : "Expected 0 or 1 mappings but got " + numMappings; + if (numMappings == 1) { + String type = in.readString(); + assert MapperService.SINGLE_MAPPING_NAME.equals(type) : "Expected [_doc] but got [" + type + "]"; + mappingsMapBuilder.put(index, new MappingMetadata(in)); + } else { + mappingsMapBuilder.put(index, MappingMetadata.EMPTY_MAPPINGS); + } + } else { + boolean hasMapping = in.readBoolean(); + mappingsMapBuilder.put(index, hasMapping ? new MappingMetadata(in) : MappingMetadata.EMPTY_MAPPINGS); } - mappingsMapBuilder.put(key, mappingEntryBuilder.build()); } mappings = mappingsMapBuilder.build(); @@ -163,11 +165,11 @@ public String[] getIndices() { return indices(); } - public ImmutableOpenMap> mappings() { + public ImmutableOpenMap mappings() { return mappings; } - public ImmutableOpenMap> getMappings() { + public ImmutableOpenMap getMappings() { return mappings(); } @@ -235,12 +237,19 @@ public String getSetting(String index, String setting) { public void writeTo(StreamOutput out) throws IOException { out.writeStringArray(indices); out.writeVInt(mappings.size()); - for (ObjectObjectCursor> indexEntry : mappings) { + for (ObjectObjectCursor indexEntry : mappings) { out.writeString(indexEntry.key); - out.writeVInt(indexEntry.value.size()); - for (ObjectObjectCursor mappingEntry : indexEntry.value) { - out.writeString(mappingEntry.key); - mappingEntry.value.writeTo(out); + if (out.getVersion().before(Version.V_2_0_0)) { + out.writeVInt(indexEntry.value == MappingMetadata.EMPTY_MAPPINGS ? 0 : 1); + if (indexEntry.value != MappingMetadata.EMPTY_MAPPINGS) { + out.writeString(MapperService.SINGLE_MAPPING_NAME); + indexEntry.value.writeTo(out); + } + } else { + out.writeBoolean(indexEntry.value != MappingMetadata.EMPTY_MAPPINGS); + if (indexEntry.value != MappingMetadata.EMPTY_MAPPINGS) { + indexEntry.value.writeTo(out); + } } } out.writeVInt(aliases.size()); @@ -286,29 +295,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } builder.endObject(); - ImmutableOpenMap indexMappings = mappings.get(index); - boolean includeTypeName = params.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY); - if (includeTypeName) { - builder.startObject("mappings"); - if (indexMappings != null) { - for (final ObjectObjectCursor typeEntry : indexMappings) { - builder.field(typeEntry.key); - builder.map(typeEntry.value.sourceAsMap()); - } - } - builder.endObject(); + MappingMetadata indexMappings = mappings.get(index); + if (indexMappings == null) { + builder.startObject("mappings").endObject(); } else { - MappingMetadata mappings = null; - for (final ObjectObjectCursor typeEntry : indexMappings) { - assert mappings == null; - mappings = typeEntry.value; - } - if (mappings == null) { - // no mappings yet - builder.startObject("mappings").endObject(); - } else { - builder.field("mappings", mappings.sourceAsMap()); - } + builder.field("mappings", indexMappings.sourceAsMap()); } builder.startObject("settings"); @@ -337,141 +328,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - private static List parseAliases(XContentParser parser) throws IOException { - List indexAliases = new ArrayList<>(); - // We start at START_OBJECT since parseIndexEntry ensures that - while (parser.nextToken() != Token.END_OBJECT) { - ensureExpectedToken(Token.FIELD_NAME, parser.currentToken(), parser); - indexAliases.add(AliasMetadata.Builder.fromXContent(parser)); - } - return indexAliases; - } - - private static ImmutableOpenMap parseMappings(XContentParser parser) throws IOException { - ImmutableOpenMap.Builder indexMappings = ImmutableOpenMap.builder(); - // We start at START_OBJECT since parseIndexEntry ensures that - while (parser.nextToken() != Token.END_OBJECT) { - ensureExpectedToken(Token.FIELD_NAME, parser.currentToken(), parser); - parser.nextToken(); - if (parser.currentToken() == Token.START_OBJECT) { - String mappingType = parser.currentName(); - indexMappings.put(mappingType, new MappingMetadata(mappingType, parser.map())); - } else if (parser.currentToken() == Token.START_ARRAY) { - parser.skipChildren(); - } - } - return indexMappings.build(); - } - - private static IndexEntry parseIndexEntry(XContentParser parser) throws IOException { - List indexAliases = null; - ImmutableOpenMap indexMappings = null; - Settings indexSettings = null; - Settings indexDefaultSettings = null; - String dataStream = null; - // We start at START_OBJECT since fromXContent ensures that - while (parser.nextToken() != Token.END_OBJECT) { - ensureExpectedToken(Token.FIELD_NAME, parser.currentToken(), parser); - parser.nextToken(); - if (parser.currentToken() == Token.START_OBJECT) { - switch (parser.currentName()) { - case "aliases": - indexAliases = parseAliases(parser); - break; - case "mappings": - indexMappings = parseMappings(parser); - break; - case "settings": - indexSettings = Settings.fromXContent(parser); - break; - case "defaults": - indexDefaultSettings = Settings.fromXContent(parser); - break; - default: - parser.skipChildren(); - } - } else if (parser.currentToken() == Token.VALUE_STRING) { - if (parser.currentName().equals("data_stream")) { - dataStream = parser.text(); - } - parser.skipChildren(); - } else if (parser.currentToken() == Token.START_ARRAY) { - parser.skipChildren(); - } - } - return new IndexEntry(indexAliases, indexMappings, indexSettings, indexDefaultSettings, dataStream); - } - - // This is just an internal container to make stuff easier for returning - private static class IndexEntry { - List indexAliases = new ArrayList<>(); - ImmutableOpenMap indexMappings = ImmutableOpenMap.of(); - Settings indexSettings = Settings.EMPTY; - Settings indexDefaultSettings = Settings.EMPTY; - String dataStream; - - IndexEntry( - List indexAliases, - ImmutableOpenMap indexMappings, - Settings indexSettings, - Settings indexDefaultSettings, - String dataStream - ) { - if (indexAliases != null) this.indexAliases = indexAliases; - if (indexMappings != null) this.indexMappings = indexMappings; - if (indexSettings != null) this.indexSettings = indexSettings; - if (indexDefaultSettings != null) this.indexDefaultSettings = indexDefaultSettings; - if (dataStream != null) this.dataStream = dataStream; - } - } - - public static GetIndexResponse fromXContent(XContentParser parser) throws IOException { - ImmutableOpenMap.Builder> aliases = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder> mappings = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder settings = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder defaultSettings = ImmutableOpenMap.builder(); - ImmutableOpenMap.Builder dataStreams = ImmutableOpenMap.builder(); - List indices = new ArrayList<>(); - - if (parser.currentToken() == null) { - parser.nextToken(); - } - ensureExpectedToken(Token.START_OBJECT, parser.currentToken(), parser); - parser.nextToken(); - - while (!parser.isClosed()) { - if (parser.currentToken() == Token.START_OBJECT) { - // we assume this is an index entry - String indexName = parser.currentName(); - indices.add(indexName); - IndexEntry indexEntry = parseIndexEntry(parser); - // make the order deterministic - CollectionUtil.timSort(indexEntry.indexAliases, Comparator.comparing(AliasMetadata::alias)); - aliases.put(indexName, Collections.unmodifiableList(indexEntry.indexAliases)); - mappings.put(indexName, indexEntry.indexMappings); - settings.put(indexName, indexEntry.indexSettings); - if (indexEntry.indexDefaultSettings.isEmpty() == false) { - defaultSettings.put(indexName, indexEntry.indexDefaultSettings); - } - if (indexEntry.dataStream != null) { - dataStreams.put(indexName, indexEntry.dataStream); - } - } else if (parser.currentToken() == Token.START_ARRAY) { - parser.skipChildren(); - } else { - parser.nextToken(); - } - } - return new GetIndexResponse( - indices.toArray(new String[0]), - mappings.build(), - aliases.build(), - settings.build(), - defaultSettings.build(), - dataStreams.build() - ); - } - @Override public String toString() { return Strings.toString(this); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/get/TransportGetIndexAction.java b/server/src/main/java/org/opensearch/action/admin/indices/get/TransportGetIndexAction.java index 872cc66f8c1ba..0cd3214307359 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/get/TransportGetIndexAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/get/TransportGetIndexAction.java @@ -102,7 +102,7 @@ protected void doMasterOperation( final ClusterState state, final ActionListener listener ) { - ImmutableOpenMap> mappingsResult = ImmutableOpenMap.of(); + ImmutableOpenMap mappingsResult = ImmutableOpenMap.of(); ImmutableOpenMap> aliasesResult = ImmutableOpenMap.of(); ImmutableOpenMap settings = ImmutableOpenMap.of(); ImmutableOpenMap defaultSettings = ImmutableOpenMap.of(); @@ -121,8 +121,7 @@ protected void doMasterOperation( case MAPPINGS: if (!doneMappings) { try { - mappingsResult = state.metadata() - .findMappings(concreteIndices, request.types(), indicesService.getFieldFilter()); + mappingsResult = state.metadata().findMappings(concreteIndices, indicesService.getFieldFilter()); doneMappings = true; } catch (IOException e) { listener.onFailure(e); diff --git a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java index 0087271147f4a..b6624acd820aa 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java @@ -33,6 +33,7 @@ package org.opensearch.action.admin.indices.mapping.get; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; +import org.opensearch.Version; import org.opensearch.action.ActionResponse; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.ParseField; @@ -42,119 +43,82 @@ import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.xcontent.ToXContentFragment; import org.opensearch.common.xcontent.XContentBuilder; -import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.rest.BaseRestHandler; +import org.opensearch.index.mapper.MapperService; import java.io.IOException; -import java.util.Map; - -import static org.opensearch.rest.BaseRestHandler.DEFAULT_INCLUDE_TYPE_NAME_POLICY; public class GetMappingsResponse extends ActionResponse implements ToXContentFragment { private static final ParseField MAPPINGS = new ParseField("mappings"); - private ImmutableOpenMap> mappings = ImmutableOpenMap.of(); + private final ImmutableOpenMap mappings; - public GetMappingsResponse(ImmutableOpenMap> mappings) { + public GetMappingsResponse(ImmutableOpenMap mappings) { this.mappings = mappings; } GetMappingsResponse(StreamInput in) throws IOException { super(in); int size = in.readVInt(); - ImmutableOpenMap.Builder> indexMapBuilder = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder indexMapBuilder = ImmutableOpenMap.builder(); for (int i = 0; i < size; i++) { - String key = in.readString(); - int valueSize = in.readVInt(); - ImmutableOpenMap.Builder typeMapBuilder = ImmutableOpenMap.builder(); - for (int j = 0; j < valueSize; j++) { - typeMapBuilder.put(in.readString(), new MappingMetadata(in)); + String index = in.readString(); + if (in.getVersion().before(Version.V_2_0_0)) { + int mappingCount = in.readVInt(); + assert mappingCount == 1 || mappingCount == 0 : "Expected 0 or 1 mappings but got " + mappingCount; + if (mappingCount == 1) { + String type = in.readString(); + assert MapperService.SINGLE_MAPPING_NAME.equals(type) : "Expected type [_doc] but got [" + type + "]"; + indexMapBuilder.put(index, new MappingMetadata(in)); + } else { + indexMapBuilder.put(index, MappingMetadata.EMPTY_MAPPINGS); + } + } else { + boolean hasMapping = in.readBoolean(); + indexMapBuilder.put(index, hasMapping ? new MappingMetadata(in) : MappingMetadata.EMPTY_MAPPINGS); } - indexMapBuilder.put(key, typeMapBuilder.build()); } mappings = indexMapBuilder.build(); } - public ImmutableOpenMap> mappings() { + public ImmutableOpenMap mappings() { return mappings; } - public ImmutableOpenMap> getMappings() { + public ImmutableOpenMap getMappings() { return mappings(); } @Override public void writeTo(StreamOutput out) throws IOException { out.writeVInt(mappings.size()); - for (ObjectObjectCursor> indexEntry : mappings) { + for (ObjectObjectCursor indexEntry : mappings) { out.writeString(indexEntry.key); - out.writeVInt(indexEntry.value.size()); - for (ObjectObjectCursor typeEntry : indexEntry.value) { - out.writeString(typeEntry.key); - typeEntry.value.writeTo(out); - } - } - } - - public static GetMappingsResponse fromXContent(XContentParser parser) throws IOException { - if (parser.currentToken() == null) { - parser.nextToken(); - } - assert parser.currentToken() == XContentParser.Token.START_OBJECT; - Map parts = parser.map(); - - ImmutableOpenMap.Builder> builder = new ImmutableOpenMap.Builder<>(); - for (Map.Entry entry : parts.entrySet()) { - final String indexName = entry.getKey(); - assert entry.getValue() instanceof Map : "expected a map as type mapping, but got: " + entry.getValue().getClass(); - final Map mapping = (Map) ((Map) entry.getValue()).get(MAPPINGS.getPreferredName()); - - ImmutableOpenMap.Builder typeBuilder = new ImmutableOpenMap.Builder<>(); - for (Map.Entry typeEntry : mapping.entrySet()) { - final String typeName = typeEntry.getKey(); - assert typeEntry.getValue() instanceof Map : "expected a map as inner type mapping, but got: " - + typeEntry.getValue().getClass(); - final Map fieldMappings = (Map) typeEntry.getValue(); - MappingMetadata mmd = new MappingMetadata(typeName, fieldMappings); - typeBuilder.put(typeName, mmd); + if (out.getVersion().before(Version.V_2_0_0)) { + out.writeVInt(indexEntry.value == MappingMetadata.EMPTY_MAPPINGS ? 0 : 1); + if (indexEntry.value != MappingMetadata.EMPTY_MAPPINGS) { + out.writeString(MapperService.SINGLE_MAPPING_NAME); + indexEntry.value.writeTo(out); + } + } else { + out.writeBoolean(indexEntry.value != MappingMetadata.EMPTY_MAPPINGS); + if (indexEntry.value != MappingMetadata.EMPTY_MAPPINGS) { + indexEntry.value.writeTo(out); + } } - builder.put(indexName, typeBuilder.build()); } - - return new GetMappingsResponse(builder.build()); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - boolean includeTypeName = params.paramAsBoolean(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY); - - for (final ObjectObjectCursor> indexEntry : getMappings()) { - builder.startObject(indexEntry.key); - { - if (includeTypeName == false) { - MappingMetadata mappings = null; - for (final ObjectObjectCursor typeEntry : indexEntry.value) { - assert mappings == null; - mappings = typeEntry.value; - } - if (mappings == null) { - // no mappings yet - builder.startObject(MAPPINGS.getPreferredName()).endObject(); - } else { - builder.field(MAPPINGS.getPreferredName(), mappings.sourceAsMap()); - } - } else { - builder.startObject(MAPPINGS.getPreferredName()); - { - for (final ObjectObjectCursor typeEntry : indexEntry.value) { - builder.field(typeEntry.key, typeEntry.value.sourceAsMap()); - } - } - builder.endObject(); - } + for (final ObjectObjectCursor indexEntry : getMappings()) { + 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/main/java/org/opensearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java index 007550f73f205..3f6cb8ed35af9 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/TransportGetMappingsAction.java @@ -91,8 +91,8 @@ protected void doMasterOperation( ) { logger.trace("serving getMapping request based on version {}", state.version()); try { - ImmutableOpenMap> result = state.metadata() - .findMappings(concreteIndices, request.types(), indicesService.getFieldFilter()); + ImmutableOpenMap result = state.metadata() + .findMappings(concreteIndices, indicesService.getFieldFilter()); listener.onResponse(new GetMappingsResponse(result)); } catch (IOException e) { listener.onFailure(e); diff --git a/server/src/main/java/org/opensearch/action/support/master/info/ClusterInfoRequest.java b/server/src/main/java/org/opensearch/action/support/master/info/ClusterInfoRequest.java index 018464a5a0cd7..0b392caa3e588 100644 --- a/server/src/main/java/org/opensearch/action/support/master/info/ClusterInfoRequest.java +++ b/server/src/main/java/org/opensearch/action/support/master/info/ClusterInfoRequest.java @@ -32,6 +32,7 @@ package org.opensearch.action.support.master.info; +import org.opensearch.Version; import org.opensearch.action.IndicesRequest; import org.opensearch.action.support.IndicesOptions; import org.opensearch.action.support.master.MasterNodeReadRequest; @@ -46,7 +47,6 @@ public abstract class ClusterInfoRequest { + public static final MappingMetadata EMPTY_MAPPINGS = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, Collections.emptyMap()); public static class Routing { @@ -88,7 +93,7 @@ public int hashCode() { private final CompressedXContent source; - private Routing routing; + private final Routing routing; public MappingMetadata(DocumentMapper docMapper) { this.type = docMapper.type(); @@ -96,6 +101,7 @@ public MappingMetadata(DocumentMapper docMapper) { this.routing = new Routing(docMapper.routingFieldMapper().required()); } + @SuppressWarnings("unchecked") public MappingMetadata(CompressedXContent mapping) { this.source = mapping; Map mappingMap = XContentHelper.convertToMap(mapping.compressedReference(), true).v2(); @@ -103,20 +109,27 @@ public MappingMetadata(CompressedXContent mapping) { throw new IllegalStateException("Can't derive type from mapping, no root type: " + mapping.string()); } this.type = mappingMap.keySet().iterator().next(); - initMappers((Map) mappingMap.get(this.type)); + this.routing = initRouting((Map) mappingMap.get(this.type)); } - public MappingMetadata(String type, Map mapping) throws IOException { + @SuppressWarnings("unchecked") + public MappingMetadata(String type, Map mapping) { this.type = type; - this.source = new CompressedXContent((builder, params) -> builder.mapContents(mapping), XContentType.JSON, ToXContent.EMPTY_PARAMS); + try { + XContentBuilder mappingBuilder = XContentFactory.jsonBuilder().map(mapping); + this.source = new CompressedXContent(BytesReference.bytes(mappingBuilder)); + } catch (IOException e) { + throw new UncheckedIOException(e); // XContent exception, should never happen + } Map withoutType = mapping; if (mapping.size() == 1 && mapping.containsKey(type)) { withoutType = (Map) mapping.get(type); } - initMappers(withoutType); + this.routing = initRouting(withoutType); } - private void initMappers(Map withoutType) { + @SuppressWarnings("unchecked") + private Routing initRouting(Map withoutType) { if (withoutType.containsKey("_routing")) { boolean required = false; Map routingNode = (Map) withoutType.get("_routing"); @@ -134,9 +147,9 @@ private void initMappers(Map withoutType) { } } } - this.routing = new Routing(required); + return new Routing(required); } else { - this.routing = Routing.EMPTY; + return Routing.EMPTY; } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index 09e3bbe0cac32..b3503f64c53f3 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -454,44 +454,26 @@ public boolean hasAliases(final String[] aliases, String[] concreteIndices) { } /** - * Finds all mappings for types and concrete indices. Types are expanded to include all types that match the glob - * patterns in the types array. Empty types array, null or {"_all"} will be expanded to all types available for - * the given indices. Only fields that match the provided field filter will be returned (default is a predicate - * that always returns true, which can be overridden via plugins) + * Finds all mappings for concrete indices. Only fields that match the provided field + * filter will be returned (default is a predicate that always returns true, which can be + * overridden via plugins) * * @see MapperPlugin#getFieldFilter() * */ - public ImmutableOpenMap> findMappings( - String[] concreteIndices, - final String[] types, - Function> fieldFilter - ) throws IOException { - assert types != null; + public ImmutableOpenMap findMappings(String[] concreteIndices, Function> fieldFilter) + throws IOException { assert concreteIndices != null; if (concreteIndices.length == 0) { return ImmutableOpenMap.of(); } - boolean isAllTypes = isAllTypes(types); - ImmutableOpenMap.Builder> indexMapBuilder = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder indexMapBuilder = ImmutableOpenMap.builder(); Iterable intersection = HppcMaps.intersection(ObjectHashSet.from(concreteIndices), indices.keys()); for (String index : intersection) { IndexMetadata indexMetadata = indices.get(index); Predicate fieldPredicate = fieldFilter.apply(index); - if (isAllTypes) { - indexMapBuilder.put(index, filterFields(indexMetadata.getMappings(), fieldPredicate)); - } else { - ImmutableOpenMap.Builder filteredMappings = ImmutableOpenMap.builder(); - for (ObjectObjectCursor cursor : indexMetadata.getMappings()) { - if (Regex.simpleMatch(types, cursor.key)) { - filteredMappings.put(cursor.key, filterFields(cursor.value, fieldPredicate)); - } - } - if (!filteredMappings.isEmpty()) { - indexMapBuilder.put(index, filteredMappings.build()); - } - } + indexMapBuilder.put(index, filterFields(indexMetadata.mapping(), fieldPredicate)); } return indexMapBuilder.build(); } @@ -514,22 +496,11 @@ public ImmutableOpenMap findDataStreams(Str return builder.build(); } - private static ImmutableOpenMap filterFields( - ImmutableOpenMap mappings, - Predicate fieldPredicate - ) throws IOException { - if (fieldPredicate == MapperPlugin.NOOP_FIELD_PREDICATE) { - return mappings; - } - ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(mappings.size()); - for (ObjectObjectCursor cursor : mappings) { - builder.put(cursor.key, filterFields(cursor.value, fieldPredicate)); - } - return builder.build(); // No types specified means return them all - } - @SuppressWarnings("unchecked") - private static MappingMetadata filterFields(MappingMetadata mappingMetadata, Predicate fieldPredicate) throws IOException { + private static MappingMetadata filterFields(MappingMetadata mappingMetadata, Predicate fieldPredicate) { + if (mappingMetadata == null) { + return MappingMetadata.EMPTY_MAPPINGS; + } if (fieldPredicate == MapperPlugin.NOOP_FIELD_PREDICATE) { return mappingMetadata; } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/get/GetIndexResponseTests.java b/server/src/test/java/org/opensearch/action/admin/indices/get/GetIndexResponseTests.java index 7b03a4bc7bc64..f712b93b409dc 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/get/GetIndexResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/get/GetIndexResponseTests.java @@ -41,26 +41,16 @@ import org.opensearch.common.io.stream.Writeable; import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; -import org.opensearch.common.xcontent.ToXContent; -import org.opensearch.common.xcontent.XContentParser; import org.opensearch.index.RandomCreateIndexGenerator; -import org.opensearch.rest.BaseRestHandler; -import org.opensearch.test.AbstractSerializingTestCase; +import org.opensearch.test.AbstractWireSerializingTestCase; -import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Locale; -import java.util.function.Predicate; -public class GetIndexResponseTests extends AbstractSerializingTestCase { - - @Override - protected GetIndexResponse doParseInstance(XContentParser parser) throws IOException { - return GetIndexResponse.fromXContent(parser); - } +public class GetIndexResponseTests extends AbstractWireSerializingTestCase { @Override protected Writeable.Reader instanceReader() { @@ -70,7 +60,7 @@ protected Writeable.Reader instanceReader() { @Override protected GetIndexResponse createTestInstance() { String[] indices = generateRandomStringArray(5, 5, false, false); - ImmutableOpenMap.Builder> mappings = ImmutableOpenMap.builder(); + ImmutableOpenMap.Builder mappings = ImmutableOpenMap.builder(); ImmutableOpenMap.Builder> aliases = ImmutableOpenMap.builder(); ImmutableOpenMap.Builder settings = ImmutableOpenMap.builder(); ImmutableOpenMap.Builder defaultSettings = ImmutableOpenMap.builder(); @@ -78,9 +68,7 @@ protected GetIndexResponse createTestInstance() { IndexScopedSettings indexScopedSettings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS; boolean includeDefaults = randomBoolean(); for (String index : indices) { - // rarely have no types - int typeCount = rarely() ? 0 : 1; - mappings.put(index, GetMappingsResponseTests.createMappingsForIndex(typeCount, true)); + mappings.put(index, GetMappingsResponseTests.createMappingsForIndex()); List aliasMetadataList = new ArrayList<>(); int aliasesNum = randomIntBetween(0, 3); @@ -111,19 +99,4 @@ protected GetIndexResponse createTestInstance() { dataStreams.build() ); } - - @Override - protected Predicate getRandomFieldsExcludeFilter() { - // we do not want to add new fields at the root (index-level), or inside the blocks - return f -> f.equals("") || f.contains(".settings") || f.contains(".defaults") || f.contains(".mappings") || f.contains(".aliases"); - } - - /** - * For xContent roundtrip testing we force the xContent output to still contain types because the parser still expects them. - * The new typeless parsing is implemented in the client side GetIndexResponse. - */ - @Override - protected ToXContent.Params getToXContentParams() { - return new ToXContent.MapParams(Collections.singletonMap(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "true")); - } } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponseTests.java b/server/src/test/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponseTests.java index 56dfbeffc21ac..5dd05789429bf 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponseTests.java @@ -32,65 +32,35 @@ package org.opensearch.action.admin.indices.mapping.get; -import com.carrotsearch.hppc.cursors.ObjectCursor; import org.opensearch.cluster.metadata.MappingMetadata; import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.io.stream.Writeable; -import org.opensearch.common.xcontent.ToXContent; -import org.opensearch.common.xcontent.ToXContent.Params; -import org.opensearch.common.xcontent.XContentParser; import org.opensearch.index.mapper.MapperService; -import org.opensearch.rest.BaseRestHandler; -import org.opensearch.test.AbstractSerializingTestCase; +import org.opensearch.test.AbstractWireSerializingTestCase; import org.opensearch.test.EqualsHashCodeTestUtils; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Objects; -public class GetMappingsResponseTests extends AbstractSerializingTestCase { - - @Override - protected boolean supportsUnknownFields() { - return false; - } +public class GetMappingsResponseTests extends AbstractWireSerializingTestCase { public void testCheckEqualsAndHashCode() { GetMappingsResponse resp = createTestInstance(); EqualsHashCodeTestUtils.checkEqualsAndHashCode(resp, r -> new GetMappingsResponse(r.mappings()), GetMappingsResponseTests::mutate); } - @Override - protected GetMappingsResponse doParseInstance(XContentParser parser) throws IOException { - return GetMappingsResponse.fromXContent(parser); - } - @Override protected Writeable.Reader instanceReader() { return GetMappingsResponse::new; } - private static GetMappingsResponse mutate(GetMappingsResponse original) throws IOException { - ImmutableOpenMap.Builder> builder = ImmutableOpenMap.builder(original.mappings()); + private static GetMappingsResponse mutate(GetMappingsResponse original) { + ImmutableOpenMap.Builder builder = ImmutableOpenMap.builder(original.mappings()); String indexKey = original.mappings().keys().iterator().next().value; + builder.put(indexKey + "1", createMappingsForIndex()); - ImmutableOpenMap.Builder typeBuilder = ImmutableOpenMap.builder(original.mappings().get(indexKey)); - final String typeKey; - Iterator> iter = original.mappings().get(indexKey).keys().iterator(); - if (iter.hasNext()) { - typeKey = iter.next().value; - } else { - typeKey = "new-type"; - } - - typeBuilder.put(typeKey, new MappingMetadata("type-" + randomAlphaOfLength(6), randomFieldMapping())); - - builder.put(indexKey, typeBuilder.build()); return new GetMappingsResponse(builder.build()); } @@ -99,48 +69,23 @@ protected GetMappingsResponse mutateInstance(GetMappingsResponse instance) throw return mutate(instance); } - public static ImmutableOpenMap createMappingsForIndex(int typeCount, boolean randomTypeName) { - List typeMappings = new ArrayList<>(typeCount); - - for (int i = 0; i < typeCount; i++) { - if (rarely() == false) { // rarely have no fields - Map mappings = new HashMap<>(); - mappings.put("field-" + i, randomFieldMapping()); - if (randomBoolean()) { - mappings.put("field2-" + i, randomFieldMapping()); - } - - try { - String typeName = MapperService.SINGLE_MAPPING_NAME; - if (randomTypeName) { - typeName = "type-" + randomAlphaOfLength(5); - } - MappingMetadata mmd = new MappingMetadata(typeName, mappings); - typeMappings.add(mmd); - } catch (IOException e) { - fail("shouldn't have failed " + e); - } + public static MappingMetadata createMappingsForIndex() { + Map mappings = new HashMap<>(); + if (rarely() == false) { // rarely have no fields + mappings.put("field", randomFieldMapping()); + if (randomBoolean()) { + mappings.put("field2", randomFieldMapping()); } + String typeName = MapperService.SINGLE_MAPPING_NAME; + return new MappingMetadata(typeName, mappings); } - ImmutableOpenMap.Builder typeBuilder = ImmutableOpenMap.builder(); - typeMappings.forEach(mmd -> typeBuilder.put(mmd.type(), mmd)); - return typeBuilder.build(); - } - - /** - * For xContent roundtrip testing we force the xContent output to still contain types because the parser - * still expects them. The new typeless parsing is implemented in the client side GetMappingsResponse. - */ - @Override - protected Params getToXContentParams() { - return new ToXContent.MapParams(Collections.singletonMap(BaseRestHandler.INCLUDE_TYPE_NAME_PARAMETER, "true")); + return new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, mappings); } @Override protected GetMappingsResponse createTestInstance() { - ImmutableOpenMap.Builder> indexBuilder = ImmutableOpenMap.builder(); - int typeCount = rarely() ? 0 : 1; - indexBuilder.put("index-" + randomAlphaOfLength(5), createMappingsForIndex(typeCount, randomBoolean())); + ImmutableOpenMap.Builder indexBuilder = ImmutableOpenMap.builder(); + indexBuilder.put("index-" + randomAlphaOfLength(5), createMappingsForIndex()); GetMappingsResponse resp = new GetMappingsResponse(indexBuilder.build()); logger.debug("--> created: {}", resp); return resp; diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java index 2ef9cf3bc9b1f..64716794bde2b 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java @@ -659,34 +659,20 @@ public void testFindMappings() throws IOException { .build(); { - ImmutableOpenMap> mappings = metadata.findMappings( - Strings.EMPTY_ARRAY, - Strings.EMPTY_ARRAY, - MapperPlugin.NOOP_FIELD_FILTER - ); - assertEquals(0, mappings.size()); - } - { - ImmutableOpenMap> mappings = metadata.findMappings( - new String[] { "index1" }, - new String[] { "notfound" }, - MapperPlugin.NOOP_FIELD_FILTER - ); + ImmutableOpenMap mappings = metadata.findMappings(Strings.EMPTY_ARRAY, MapperPlugin.NOOP_FIELD_FILTER); assertEquals(0, mappings.size()); } { - ImmutableOpenMap> mappings = metadata.findMappings( + ImmutableOpenMap mappings = metadata.findMappings( new String[] { "index1" }, - Strings.EMPTY_ARRAY, MapperPlugin.NOOP_FIELD_FILTER ); assertEquals(1, mappings.size()); assertIndexMappingsNotFiltered(mappings, "index1"); } { - ImmutableOpenMap> mappings = metadata.findMappings( + ImmutableOpenMap mappings = metadata.findMappings( new String[] { "index1", "index2" }, - new String[] { randomBoolean() ? "_doc" : "_all" }, MapperPlugin.NOOP_FIELD_FILTER ); assertEquals(2, mappings.size()); @@ -715,43 +701,19 @@ public void testFindMappingsNoOpFilters() throws IOException { .build(); { - ImmutableOpenMap> mappings = metadata.findMappings( - new String[] { "index1" }, - randomBoolean() ? Strings.EMPTY_ARRAY : new String[] { "_all" }, - MapperPlugin.NOOP_FIELD_FILTER - ); - ImmutableOpenMap index1 = mappings.get("index1"); - MappingMetadata mappingMetadata = index1.get("_doc"); - assertSame(originalMappingMetadata, mappingMetadata); - } - { - ImmutableOpenMap> mappings = metadata.findMappings( - new String[] { "index1" }, - randomBoolean() ? Strings.EMPTY_ARRAY : new String[] { "_all" }, - index -> field -> randomBoolean() - ); - ImmutableOpenMap index1 = mappings.get("index1"); - MappingMetadata mappingMetadata = index1.get("_doc"); - assertNotSame(originalMappingMetadata, mappingMetadata); - } - { - ImmutableOpenMap> mappings = metadata.findMappings( + ImmutableOpenMap mappings = metadata.findMappings( new String[] { "index1" }, - new String[] { "_doc" }, MapperPlugin.NOOP_FIELD_FILTER ); - ImmutableOpenMap index1 = mappings.get("index1"); - MappingMetadata mappingMetadata = index1.get("_doc"); + MappingMetadata mappingMetadata = mappings.get("index1"); assertSame(originalMappingMetadata, mappingMetadata); } { - ImmutableOpenMap> mappings = metadata.findMappings( + ImmutableOpenMap mappings = metadata.findMappings( new String[] { "index1" }, - new String[] { "_doc" }, index -> field -> randomBoolean() ); - ImmutableOpenMap index1 = mappings.get("index1"); - MappingMetadata mappingMetadata = index1.get("_doc"); + MappingMetadata mappingMetadata = mappings.get("index1"); assertNotSame(originalMappingMetadata, mappingMetadata); } } @@ -802,9 +764,8 @@ public void testFindMappingsWithFilters() throws IOException { .build(); { - ImmutableOpenMap> mappings = metadata.findMappings( + ImmutableOpenMap mappings = metadata.findMappings( new String[] { "index1", "index2", "index3" }, - new String[] { "_doc" }, index -> { if (index.equals("index1")) { return field -> field.startsWith("name.") == false @@ -822,11 +783,7 @@ public void testFindMappingsWithFilters() throws IOException { assertIndexMappingsNoFields(mappings, "index2"); assertIndexMappingsNotFiltered(mappings, "index3"); - ImmutableOpenMap index1Mappings = mappings.get("index1"); - assertNotNull(index1Mappings); - - assertEquals(1, index1Mappings.size()); - MappingMetadata docMapping = index1Mappings.get("_doc"); + MappingMetadata docMapping = mappings.get("index1"); assertNotNull(docMapping); Map sourceAsMap = docMapping.getSourceAsMap(); @@ -868,17 +825,14 @@ public void testFindMappingsWithFilters() throws IOException { } { - ImmutableOpenMap> mappings = metadata.findMappings( + ImmutableOpenMap mappings = metadata.findMappings( new String[] { "index1", "index2", "index3" }, - new String[] { "_doc" }, index -> field -> (index.equals("index3") && field.endsWith("keyword")) ); assertIndexMappingsNoFields(mappings, "index1"); assertIndexMappingsNoFields(mappings, "index2"); - ImmutableOpenMap index3 = mappings.get("index3"); - assertEquals(1, index3.size()); - MappingMetadata mappingMetadata = index3.get("_doc"); + MappingMetadata mappingMetadata = mappings.get("index3"); Map sourceAsMap = mappingMetadata.getSourceAsMap(); assertEquals(3, sourceAsMap.size()); assertTrue(sourceAsMap.containsKey("_routing")); @@ -906,9 +860,8 @@ public void testFindMappingsWithFilters() throws IOException { } { - ImmutableOpenMap> mappings = metadata.findMappings( + ImmutableOpenMap mappings = metadata.findMappings( new String[] { "index1", "index2", "index3" }, - new String[] { "_doc" }, index -> field -> (index.equals("index2")) ); @@ -928,14 +881,8 @@ private static IndexMetadata.Builder buildIndexMetadata(String name, String alia } @SuppressWarnings("unchecked") - private static void assertIndexMappingsNoFields( - ImmutableOpenMap> mappings, - String index - ) { - ImmutableOpenMap indexMappings = mappings.get(index); - assertNotNull(indexMappings); - assertEquals(1, indexMappings.size()); - MappingMetadata docMapping = indexMappings.get("_doc"); + private static void assertIndexMappingsNoFields(ImmutableOpenMap mappings, String index) { + MappingMetadata docMapping = mappings.get(index); assertNotNull(docMapping); Map sourceAsMap = docMapping.getSourceAsMap(); assertEquals(3, sourceAsMap.size()); @@ -946,15 +893,8 @@ private static void assertIndexMappingsNoFields( } @SuppressWarnings("unchecked") - private static void assertIndexMappingsNotFiltered( - ImmutableOpenMap> mappings, - String index - ) { - ImmutableOpenMap indexMappings = mappings.get(index); - assertNotNull(indexMappings); - - assertEquals(1, indexMappings.size()); - MappingMetadata docMapping = indexMappings.get("_doc"); + private static void assertIndexMappingsNotFiltered(ImmutableOpenMap mappings, String index) { + MappingMetadata docMapping = mappings.get(index); assertNotNull(docMapping); Map sourceAsMap = docMapping.getSourceAsMap(); diff --git a/server/src/test/java/org/opensearch/index/mapper/FieldFilterMapperPluginTests.java b/server/src/test/java/org/opensearch/index/mapper/FieldFilterMapperPluginTests.java index 87e3ba253bfe3..f388fb0d18db1 100644 --- a/server/src/test/java/org/opensearch/index/mapper/FieldFilterMapperPluginTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/FieldFilterMapperPluginTests.java @@ -102,8 +102,8 @@ public void testGetFieldMappings() { // double check that submitting the filtered mappings to an unfiltered index leads to the same get field mappings output // as the one coming from a filtered index with same mappings GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("filtered").get(); - ImmutableOpenMap filtered = getMappingsResponse.getMappings().get("filtered"); - assertAcked(client().admin().indices().prepareCreate("test").addMapping("_doc", filtered.get("_doc").getSourceAsMap())); + MappingMetadata filtered = getMappingsResponse.getMappings().get("filtered"); + assertAcked(client().admin().indices().prepareCreate("test").addMapping("_doc", filtered.getSourceAsMap())); GetFieldMappingsResponse response = client().admin().indices().prepareGetFieldMappings("test").setFields("*").get(); assertEquals(1, response.mappings().size()); assertFieldMappings(response.mappings().get("test"), FILTERED_FLAT_FIELDS); @@ -121,8 +121,8 @@ public void testFieldCapabilities() { // double check that submitting the filtered mappings to an unfiltered index leads to the same field_caps output // as the one coming from a filtered index with same mappings GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("filtered").get(); - ImmutableOpenMap filteredMapping = getMappingsResponse.getMappings().get("filtered"); - assertAcked(client().admin().indices().prepareCreate("test").addMapping("_doc", filteredMapping.get("_doc").getSourceAsMap())); + MappingMetadata filteredMapping = getMappingsResponse.getMappings().get("filtered"); + assertAcked(client().admin().indices().prepareCreate("test").addMapping("_doc", filteredMapping.getSourceAsMap())); FieldCapabilitiesResponse test = client().fieldCaps(new FieldCapabilitiesRequest().fields("*").indices("test")).actionGet(); // properties.value is an object field in the new mapping filteredFields.add("properties.value"); @@ -161,12 +161,12 @@ private static void assertFieldMappings( assertEquals("Some unexpected fields were returned: " + fields.keySet(), 0, fields.size()); } - private void assertExpectedMappings(ImmutableOpenMap> mappings) { + private void assertExpectedMappings(ImmutableOpenMap mappings) { assertEquals(2, mappings.size()); assertNotFiltered(mappings.get("index1")); - ImmutableOpenMap filtered = mappings.get("filtered"); + MappingMetadata filtered = mappings.get("filtered"); assertFiltered(filtered); - assertMappingsAreValid(filtered.get("_doc").getSourceAsMap()); + assertMappingsAreValid(filtered.getSourceAsMap()); } private void assertMappingsAreValid(Map sourceAsMap) { @@ -179,9 +179,7 @@ private void assertMappingsAreValid(Map sourceAsMap) { } @SuppressWarnings("unchecked") - private static void assertFiltered(ImmutableOpenMap mappings) { - assertEquals(1, mappings.size()); - MappingMetadata mappingMetadata = mappings.get("_doc"); + private static void assertFiltered(MappingMetadata mappingMetadata) { assertNotNull(mappingMetadata); Map sourceAsMap = mappingMetadata.getSourceAsMap(); assertEquals(4, sourceAsMap.size()); @@ -226,9 +224,7 @@ private static void assertFiltered(ImmutableOpenMap map } @SuppressWarnings("unchecked") - private static void assertNotFiltered(ImmutableOpenMap mappings) { - assertEquals(1, mappings.size()); - MappingMetadata mappingMetadata = mappings.get("_doc"); + private static void assertNotFiltered(MappingMetadata mappingMetadata) { assertNotNull(mappingMetadata); Map sourceAsMap = mappingMetadata.getSourceAsMap(); assertEquals(4, sourceAsMap.size()); From 5fad2035dd2470a2a9d810d112781703aeb2e2c2 Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Fri, 25 Feb 2022 14:45:20 -0600 Subject: [PATCH 2/5] remove invalid yml test Signed-off-by: Nicholas Walter Knize --- .../test/indices.get/11_basic_with_types.yml | 70 ------------------- 1 file changed, 70 deletions(-) delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/indices.get/11_basic_with_types.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get/11_basic_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get/11_basic_with_types.yml deleted file mode 100644 index 85267f49b1317..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.get/11_basic_with_types.yml +++ /dev/null @@ -1,70 +0,0 @@ ---- -setup: - - - do: - indices.create: - include_type_name: true - index: test_index - body: - aliases: - test_alias: {} - test_blias: {} - mappings: - type_1: {} - settings: - number_of_shards: 1 - number_of_replicas: 1 - - - do: - indices.create: - index: test_index_2 - body: - settings: - number_of_shards: 1 - number_of_replicas: 2 - aliases: - test_alias: {} - test_blias: {} - - - do: - indices.create: - index: test_index_3 - body: - aliases: - test_alias: {} - test_blias: {} - - - do: - indices.close: - index: test_index_3 - - - do: - cluster.health: - wait_for_status: yellow - ---- -"Test include_type_name": - - do: - indices.get: - include_type_name: true - index: test_index - - - is_true: test_index.mappings - - is_true: test_index.mappings.type_1 - - - do: - indices.get: - include_type_name: false - index: test_index - - - is_true: test_index.mappings - - is_false: test_index.mappings.type_1 - ---- -"Test include_type_name dafaults to false": - - do: - indices.get: - index: test_index - - - is_true: test_index.mappings - - is_false: test_index.mappings.type_1 From f497d095d737f5f48c5d8252383b9310a60bbf32 Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Tue, 1 Mar 2022 16:41:09 -0600 Subject: [PATCH 3/5] fix cluster state check in OpenSearchIntegTestCase CompressedXContent's DeflateCompresser uses a synced flush that could end up with different compressed representations for the same data. We compare string length of clusterstate to ensure similarity Signed-off-by: Nicholas Walter Knize --- .../action/bulk/TransportShardBulkAction.java | 2 +- .../common/compress/CompressedXContent.java | 13 ++++++++----- .../org/opensearch/index/mapper/DocumentMapper.java | 2 +- .../opensearch/test/OpenSearchIntegTestCase.java | 9 ++++++--- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java b/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java index ed407bd37d684..b97219782e975 100644 --- a/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java +++ b/server/src/main/java/org/opensearch/action/bulk/TransportShardBulkAction.java @@ -371,7 +371,7 @@ static boolean executeBulkItemRequest( primary.mapperService() .merge( MapperService.SINGLE_MAPPING_NAME, - new CompressedXContent(result.getRequiredMappingUpdate(), XContentType.JSON, ToXContent.EMPTY_PARAMS), + new CompressedXContent(result.getRequiredMappingUpdate(), ToXContent.EMPTY_PARAMS), MapperService.MergeReason.MAPPING_UPDATE_PREFLIGHT ); } catch (Exception e) { diff --git a/server/src/main/java/org/opensearch/common/compress/CompressedXContent.java b/server/src/main/java/org/opensearch/common/compress/CompressedXContent.java index e883b3739c9da..f15e213b9a773 100644 --- a/server/src/main/java/org/opensearch/common/compress/CompressedXContent.java +++ b/server/src/main/java/org/opensearch/common/compress/CompressedXContent.java @@ -41,7 +41,6 @@ import org.opensearch.common.xcontent.ToXContent; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentFactory; -import org.opensearch.common.xcontent.XContentType; import java.io.IOException; import java.io.OutputStream; @@ -82,15 +81,19 @@ private CompressedXContent(byte[] compressed, int crc32) { /** * Create a {@link CompressedXContent} out of a {@link ToXContent} instance. */ - public CompressedXContent(ToXContent xcontent, XContentType type, ToXContent.Params params) throws IOException { + public CompressedXContent(ToXContent xcontent, ToXContent.Params params) throws IOException { BytesStreamOutput bStream = new BytesStreamOutput(); OutputStream compressedStream = CompressorFactory.COMPRESSOR.threadLocalOutputStream(bStream); CRC32 crc32 = new CRC32(); OutputStream checkedStream = new CheckedOutputStream(compressedStream, crc32); - try (XContentBuilder builder = XContentFactory.contentBuilder(type, checkedStream)) { - builder.startObject(); + try (XContentBuilder builder = XContentFactory.jsonBuilder(checkedStream)) { + if (xcontent.isFragment()) { + builder.startObject(); + } xcontent.toXContent(builder, params); - builder.endObject(); + if (xcontent.isFragment()) { + builder.endObject(); + } } this.bytes = BytesReference.toBytes(bStream.bytes()); this.crc32 = (int) crc32.getValue(); diff --git a/server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java index d85c18e272793..c6724d1b3063e 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java @@ -153,7 +153,7 @@ public DocumentMapper(MapperService mapperService, Mapping mapping) { this.fieldMappers = MappingLookup.fromMapping(this.mapping, indexAnalyzers.getDefaultIndexAnalyzer()); try { - mappingSource = new CompressedXContent(this, XContentType.JSON, ToXContent.EMPTY_PARAMS); + mappingSource = new CompressedXContent(this, ToXContent.EMPTY_PARAMS); } catch (Exception e) { throw new OpenSearchGenerationException("failed to serialize source for type [" + type + "]", e); } diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index e4e49a8ef310d..b9ee655dcc979 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -1096,7 +1096,7 @@ protected void ensureClusterStateConsistency() throws IOException { // remove local node reference masterClusterState = ClusterState.Builder.fromBytes(masterClusterStateBytes, null, namedWriteableRegistry); Map masterStateMap = convertToMap(masterClusterState); - int masterClusterStateSize = ClusterState.Builder.toBytes(masterClusterState).length; + int masterClusterStateSize = masterClusterState.toString().length(); String masterId = masterClusterState.nodes().getMasterNodeId(); for (Client client : cluster().getClients()) { ClusterState localClusterState = client.admin().cluster().prepareState().all().setLocal(true).get().getState(); @@ -1104,7 +1104,7 @@ protected void ensureClusterStateConsistency() throws IOException { // remove local node reference localClusterState = ClusterState.Builder.fromBytes(localClusterStateBytes, null, namedWriteableRegistry); final Map localStateMap = convertToMap(localClusterState); - final int localClusterStateSize = ClusterState.Builder.toBytes(localClusterState).length; + final int localClusterStateSize = localClusterState.toString().length(); // Check that the non-master node has the same version of the cluster state as the master and // that the master node matches the master (otherwise there is no requirement for the cluster state to match) if (masterClusterState.version() == localClusterState.version() @@ -1112,7 +1112,10 @@ protected void ensureClusterStateConsistency() throws IOException { try { assertEquals("cluster state UUID does not match", masterClusterState.stateUUID(), localClusterState.stateUUID()); // We cannot compare serialization bytes since serialization order of maps is not guaranteed - // but we can compare serialization sizes - they should be the same + // We also cannot compare byte array size because CompressedXContent's DeflateCompressor uses + // a synced flush that can affect the size of the compressed byte array + // (see: DeflateCompressedXContentTests#testDifferentCompressedRepresentation for an example) + // instead we compare the string length of cluster state - they should be the same assertEquals("cluster state size does not match", masterClusterStateSize, localClusterStateSize); // Compare JSON serialization assertNull( From ab637c617f0282776d58ccffce407647245e7e21 Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Wed, 2 Mar 2022 07:42:43 -0600 Subject: [PATCH 4/5] cleanup from PR feedback Signed-off-by: Nicholas Walter Knize --- .../admin/indices/get/GetIndexResponse.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java index 730ba018c3471..4465dc88fe87d 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/get/GetIndexResponse.java @@ -104,17 +104,20 @@ public GetIndexResponse( String index = in.readString(); if (in.getVersion().before(Version.V_2_0_0)) { int numMappings = in.readVInt(); - assert numMappings == 0 || numMappings == 1 : "Expected 0 or 1 mappings but got " + numMappings; - if (numMappings == 1) { + if (numMappings == 0) { + mappingsMapBuilder.put(index, MappingMetadata.EMPTY_MAPPINGS); + } else if (numMappings == 1) { String type = in.readString(); - assert MapperService.SINGLE_MAPPING_NAME.equals(type) : "Expected [_doc] but got [" + type + "]"; + if (MapperService.SINGLE_MAPPING_NAME.equals(type) == false) { + throw new IllegalStateException("Expected " + MapperService.SINGLE_MAPPING_NAME + " but got [" + type + "]"); + } mappingsMapBuilder.put(index, new MappingMetadata(in)); } else { - mappingsMapBuilder.put(index, MappingMetadata.EMPTY_MAPPINGS); + throw new IllegalStateException("Expected 0 or 1 mappings but got: " + numMappings); } } else { - boolean hasMapping = in.readBoolean(); - mappingsMapBuilder.put(index, hasMapping ? new MappingMetadata(in) : MappingMetadata.EMPTY_MAPPINGS); + final MappingMetadata metadata = in.readOptionalWriteable(MappingMetadata::new); + mappingsMapBuilder.put(index, metadata != null ? metadata : MappingMetadata.EMPTY_MAPPINGS); } } mappings = mappingsMapBuilder.build(); @@ -246,10 +249,7 @@ public void writeTo(StreamOutput out) throws IOException { indexEntry.value.writeTo(out); } } else { - out.writeBoolean(indexEntry.value != MappingMetadata.EMPTY_MAPPINGS); - if (indexEntry.value != MappingMetadata.EMPTY_MAPPINGS) { - indexEntry.value.writeTo(out); - } + out.writeOptionalWriteable(indexEntry.value); } } out.writeVInt(aliases.size()); From 7567f4c8588f6b434938d1977080e97cfe47b6ba Mon Sep 17 00:00:00 2001 From: Nicholas Walter Knize Date: Wed, 2 Mar 2022 12:35:50 -0600 Subject: [PATCH 5/5] clean up GettMappingsResponse Signed-off-by: Nicholas Walter Knize --- .../indices/mapping/get/GetMappingsResponse.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java index b6624acd820aa..d203a5e6a45fe 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/mapping/get/GetMappingsResponse.java @@ -65,13 +65,16 @@ public GetMappingsResponse(ImmutableOpenMap mappings) { String index = in.readString(); if (in.getVersion().before(Version.V_2_0_0)) { int mappingCount = in.readVInt(); - assert mappingCount == 1 || mappingCount == 0 : "Expected 0 or 1 mappings but got " + mappingCount; - if (mappingCount == 1) { + if (mappingCount == 0) { + indexMapBuilder.put(index, MappingMetadata.EMPTY_MAPPINGS); + } else if (mappingCount == 1) { String type = in.readString(); - assert MapperService.SINGLE_MAPPING_NAME.equals(type) : "Expected type [_doc] but got [" + type + "]"; + if (MapperService.SINGLE_MAPPING_NAME.equals(type) == false) { + throw new IllegalStateException("Expected " + MapperService.SINGLE_MAPPING_NAME + " but got [" + type + "]"); + } indexMapBuilder.put(index, new MappingMetadata(in)); } else { - indexMapBuilder.put(index, MappingMetadata.EMPTY_MAPPINGS); + throw new IllegalStateException("Expected 0 or 1 mappings but got: " + mappingCount); } } else { boolean hasMapping = in.readBoolean(); @@ -101,10 +104,7 @@ public void writeTo(StreamOutput out) throws IOException { indexEntry.value.writeTo(out); } } else { - out.writeBoolean(indexEntry.value != MappingMetadata.EMPTY_MAPPINGS); - if (indexEntry.value != MappingMetadata.EMPTY_MAPPINGS) { - indexEntry.value.writeTo(out); - } + out.writeOptionalWriteable(indexEntry.value); } } }