From 4a0fe5a257552094672fc49133e68ddb0b73a41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 17 Jul 2018 09:09:03 +0200 Subject: [PATCH] Fix put mappings java API documentation (#31955) The current docs of the put-mapping Java API is currently broken. It its current form, it creates an index and uses the whole mapping definition given as a JSON string as the type name. Since we didn't check the index created in the IndicesDocumentationIT so far this went unnoticed. This change adds test to catch this error to the documentation test, changes the documentation so it works correctly now and adds an input validation to PutMappingRequest#buildFromSimplifiedDef() which was used internally to reject calls where no mapping definition is given. Closes #31906 --- .../admin/indices/put-mapping.asciidoc | 11 ++++-- .../mapping/put/PutMappingRequest.java | 11 +++--- .../mapping/put/PutMappingRequestTests.java | 6 +++- .../documentation/IndicesDocumentationIT.java | 35 ++++++++++++++----- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/docs/java-api/admin/indices/put-mapping.asciidoc b/docs/java-api/admin/indices/put-mapping.asciidoc index a70e055f3a0ce..d89c73bb1a261 100644 --- a/docs/java-api/admin/indices/put-mapping.asciidoc +++ b/docs/java-api/admin/indices/put-mapping.asciidoc @@ -1,17 +1,22 @@ [[java-admin-indices-put-mapping]] ==== Put Mapping -The PUT mapping API allows you to add a new type while creating an index: +You can add mappings for a new type at index creation time: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- include-tagged::{client-tests}/IndicesDocumentationIT.java[index-with-mapping] -------------------------------------------------- <1> <> called `twitter` -<2> It also adds a `tweet` mapping type. +<2> Add a `tweet` type with a field called `message` that has the datatype `text`. +There are several variants of the above `addMapping` method, some taking an +`XContentBuilder` or a `Map` with the mapping definition as arguments. Make sure +to check the javadocs to pick the simplest one for your use case. -The PUT mapping API also allows to add a new type to an existing index: +The PUT mapping API also allows to specify the mapping of a type after index +creation. In this case you can provide the mapping as a String similar to the +Rest API syntax: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java index 916a075b7c81b..7631dc3d1b2c4 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java @@ -185,10 +185,13 @@ public PutMappingRequest source(Object... source) { } /** - * @param type the mapping type - * @param source consisting of field/properties pairs (e.g. "field1", - * "type=string,store=true"). If the number of arguments is not - * divisible by two an {@link IllegalArgumentException} is thrown + * @param type + * the mapping type + * @param source + * consisting of field/properties pairs (e.g. "field1", + * "type=string,store=true") + * @throws IllegalArgumentException + * if the number of the source arguments is not divisible by two * @return the mappings definition */ public static XContentBuilder buildFromSimplifiedDef(String type, Object... source) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java index e816b08187f1b..eac9fb53fddde 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequestTests.java @@ -76,8 +76,12 @@ public void testValidation() { " concrete index: [[foo/bar]] and indices: [myindex];"); } + /** + * Test that {@link PutMappingRequest#buildFromSimplifiedDef(String, Object...)} + * rejects inputs where the {@code Object...} varargs of field name and properties are not + * paired correctly + */ public void testBuildFromSimplifiedDef() { - // test that method rejects input where input varargs fieldname/properites are not paired correctly IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PutMappingRequest.buildFromSimplifiedDef("type", "only_field")); assertEquals("mapping source must be pairs of fieldnames and properties definition.", e.getMessage()); diff --git a/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java b/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java index 064702170d5bb..e5df229cd98a9 100644 --- a/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java +++ b/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java @@ -19,10 +19,19 @@ package org.elasticsearch.client.documentation; +import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.metadata.MappingMetaData; +import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESIntegTestCase; +import java.util.HashMap; +import java.util.Map; + +import static java.util.Collections.singletonMap; +import static org.hamcrest.Matchers.instanceOf; + /** * This class is used to generate the Java indices administration documentation. * You need to wrap your code between two tags like: @@ -48,16 +57,14 @@ public void testPutMappingDocumentation() throws Exception { Client client = client(); // tag::index-with-mapping - client.admin().indices().prepareCreate("twitter") // <1> - .addMapping("\"tweet\": {\n" + // <2> - " \"properties\": {\n" + - " \"message\": {\n" + - " \"type\": \"text\"\n" + - " }\n" + - " }\n" + - "}") + client.admin().indices().prepareCreate("twitter") // <1> + .addMapping("tweet", "message", "type=text") // <2> .get(); // 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("tweet"), instanceOf(MappingMetaData.class)); // we need to delete in order to create a fresh new index with another type client.admin().indices().prepareDelete("twitter").get(); @@ -89,6 +96,11 @@ public void testPutMappingDocumentation() throws Exception { "}", XContentType.JSON) .get(); // 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("user").getSourceAsMap()); // tag::putMapping-request-source-append client.admin().indices().preparePutMapping("twitter") // <1> @@ -102,6 +114,13 @@ public void testPutMappingDocumentation() throws Exception { "}", XContentType.JSON) .get(); // end::putMapping-request-source-append + getMappingsResponse = client.admin().indices().prepareGetMappings("twitter").get(); + assertEquals(1, getMappingsResponse.getMappings().size()); + indexMapping = getMappingsResponse.getMappings().get("twitter"); + Map> expected = new HashMap<>(); + expected.put("name", singletonMap("type", "text")); + expected.put("user_name", singletonMap("type", "text")); + assertEquals(expected, indexMapping.get("user").getSourceAsMap().get("properties")); } }