From abd314b426be1fba481792a2b9272b72d2ddb924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 10 Jul 2018 21:42:29 +0200 Subject: [PATCH 1/5] [Docs] Fix put mappings java API documentation 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 | 14 +++++---- .../mapping/put/PutMappingRequestTests.java | 8 ++++- .../documentation/IndicesDocumentationIT.java | 30 ++++++++++++++----- 4 files changed, 46 insertions(+), 17 deletions(-) diff --git a/docs/java-api/admin/indices/put-mapping.asciidoc b/docs/java-api/admin/indices/put-mapping.asciidoc index 3e931dfd7b7e7..694c1ab103aec 100644 --- a/docs/java-api/admin/indices/put-mapping.asciidoc +++ b/docs/java-api/admin/indices/put-mapping.asciidoc @@ -2,17 +2,22 @@ ==== 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 already while creating an index: ["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 dc201b38c3bee..1d834bd11f1c4 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 @@ -184,14 +184,18 @@ 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 0 or not divisible by + * two * @return the mappings definition */ public static XContentBuilder buildFromSimplifiedDef(String type, Object... source) { - if (source.length % 2 != 0) { + if (source.length == 0 || source.length % 2 != 0) { throw new IllegalArgumentException("mapping source must be pairs of fieldnames and properties definition."); } try { 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 be44d790b4004..a31afb6a6b77e 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,11 +76,17 @@ public void testValidation() { " concrete index: [[foo/bar]] and indices: [myindex];"); } + /** + * Test that method rejects input where input varargs fieldname/properites are not paired correctly + * or input is empty. + */ 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()); + e = expectThrows(IllegalArgumentException.class, + () -> PutMappingRequest.buildFromSimplifiedDef("type")); + assertEquals("mapping source must be pairs of fieldnames and properties definition.", e.getMessage()); } public void testPutMappingRequestSerialization() throws IOException { 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..b93416172e0e5 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,16 @@ 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 static org.hamcrest.Matchers.equalTo; +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 +54,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 +93,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"); + assertThat(indexMapping.get("user"), instanceOf(MappingMetaData.class)); + assertThat(indexMapping.get("user").getSourceAsMap().toString(), equalTo("{properties={name={type=text}}}")); // tag::putMapping-request-source-append client.admin().indices().preparePutMapping("twitter") // <1> @@ -102,6 +111,11 @@ 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"); + assertThat(indexMapping.get("user"), instanceOf(MappingMetaData.class)); + assertThat(indexMapping.get("user").getSourceAsMap().toString(), equalTo("{properties={name={type=text}, user_name={type=text}}}")); } } From 48d993a6431f2a8f6faaf6770bb16a70f1684c4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 11 Jul 2018 14:56:10 +0200 Subject: [PATCH 2/5] Remove restriction on non-empty args in PutMappingRequest.buildFromSimplifiedDef --- .../action/admin/indices/mapping/put/PutMappingRequest.java | 5 ++--- .../admin/indices/mapping/put/PutMappingRequestTests.java | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) 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 1d834bd11f1c4..3429b35073ca2 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 @@ -190,12 +190,11 @@ public PutMappingRequest source(Object... source) { * consisting of field/properties pairs (e.g. "field1", * "type=string,store=true") * @throws IllegalArgumentException - * if the number of the source arguments is 0 or not divisible by - * two + * if the number of the source arguments is not divisible by two * @return the mappings definition */ public static XContentBuilder buildFromSimplifiedDef(String type, Object... source) { - if (source.length == 0 || source.length % 2 != 0) { + if (source.length % 2 != 0) { throw new IllegalArgumentException("mapping source must be pairs of fieldnames and properties definition."); } try { 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 a31afb6a6b77e..f6322741d4340 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 @@ -84,9 +84,6 @@ public void testBuildFromSimplifiedDef() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PutMappingRequest.buildFromSimplifiedDef("type", "only_field")); assertEquals("mapping source must be pairs of fieldnames and properties definition.", e.getMessage()); - e = expectThrows(IllegalArgumentException.class, - () -> PutMappingRequest.buildFromSimplifiedDef("type")); - assertEquals("mapping source must be pairs of fieldnames and properties definition.", e.getMessage()); } public void testPutMappingRequestSerialization() throws IOException { From a1662d750b2e309e5719c2a2dff6e1d04f057d72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 11 Jul 2018 15:21:10 +0200 Subject: [PATCH 3/5] iter --- .../admin/indices/mapping/put/PutMappingRequestTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 f6322741d4340..86c2b67be9c54 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 @@ -77,8 +77,9 @@ public void testValidation() { } /** - * Test that method rejects input where input varargs fieldname/properites are not paired correctly - * or input is empty. + * 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() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, From 34514b25080a603b2a616e378d7fa3d2dbdc0e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 16 Jul 2018 13:39:06 +0200 Subject: [PATCH 4/5] iter --- docs/java-api/admin/indices/put-mapping.asciidoc | 2 +- .../client/documentation/IndicesDocumentationIT.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/java-api/admin/indices/put-mapping.asciidoc b/docs/java-api/admin/indices/put-mapping.asciidoc index 694c1ab103aec..8bdcb4916976f 100644 --- a/docs/java-api/admin/indices/put-mapping.asciidoc +++ b/docs/java-api/admin/indices/put-mapping.asciidoc @@ -2,7 +2,7 @@ ==== Put Mapping -You can add mappings for a new type already while creating an index: +You can add mappings for a new type at index creation time: ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- 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 b93416172e0e5..930989745ead7 100644 --- a/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java +++ b/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java @@ -26,7 +26,7 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESIntegTestCase; -import static org.hamcrest.Matchers.equalTo; +import static java.util.Collections.singletonMap; import static org.hamcrest.Matchers.instanceOf; /** @@ -96,8 +96,8 @@ public void testPutMappingDocumentation() throws Exception { getMappingsResponse = client.admin().indices().prepareGetMappings("twitter").get(); assertEquals(1, getMappingsResponse.getMappings().size()); indexMapping = getMappingsResponse.getMappings().get("twitter"); - assertThat(indexMapping.get("user"), instanceOf(MappingMetaData.class)); - assertThat(indexMapping.get("user").getSourceAsMap().toString(), equalTo("{properties={name={type=text}}}")); + assertEquals(indexMapping.get("user").getSourceAsMap(), + singletonMap("properties", singletonMap("name", singletonMap("type", "text")))); // tag::putMapping-request-source-append client.admin().indices().preparePutMapping("twitter") // <1> @@ -114,8 +114,8 @@ public void testPutMappingDocumentation() throws Exception { getMappingsResponse = client.admin().indices().prepareGetMappings("twitter").get(); assertEquals(1, getMappingsResponse.getMappings().size()); indexMapping = getMappingsResponse.getMappings().get("twitter"); - assertThat(indexMapping.get("user"), instanceOf(MappingMetaData.class)); - assertThat(indexMapping.get("user").getSourceAsMap().toString(), equalTo("{properties={name={type=text}, user_name={type=text}}}")); + assertEquals(indexMapping.get("user").getSourceAsMap(), + singletonMap("properties", singletonMap("user_name", singletonMap("type", "text")))); } } From a2a7f0c22949f32e82213796082dd57903d461a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 16 Jul 2018 13:48:05 +0200 Subject: [PATCH 5/5] iter --- .../documentation/IndicesDocumentationIT.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) 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 930989745ead7..e5df229cd98a9 100644 --- a/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java +++ b/server/src/test/java/org/elasticsearch/client/documentation/IndicesDocumentationIT.java @@ -26,6 +26,9 @@ 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; @@ -96,8 +99,8 @@ public void testPutMappingDocumentation() throws Exception { getMappingsResponse = client.admin().indices().prepareGetMappings("twitter").get(); assertEquals(1, getMappingsResponse.getMappings().size()); indexMapping = getMappingsResponse.getMappings().get("twitter"); - assertEquals(indexMapping.get("user").getSourceAsMap(), - singletonMap("properties", singletonMap("name", singletonMap("type", "text")))); + assertEquals(singletonMap("properties", singletonMap("name", singletonMap("type", "text"))), + indexMapping.get("user").getSourceAsMap()); // tag::putMapping-request-source-append client.admin().indices().preparePutMapping("twitter") // <1> @@ -114,8 +117,10 @@ public void testPutMappingDocumentation() throws Exception { getMappingsResponse = client.admin().indices().prepareGetMappings("twitter").get(); assertEquals(1, getMappingsResponse.getMappings().size()); indexMapping = getMappingsResponse.getMappings().get("twitter"); - assertEquals(indexMapping.get("user").getSourceAsMap(), - singletonMap("properties", singletonMap("user_name", singletonMap("type", "text")))); + 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")); } }