Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Put Mapping API to high-level Rest client (#27205) #27869

Merged
merged 10 commits into from
Jan 23, 2018
Merged

Added Put Mapping API to high-level Rest client (#27205) #27869

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
232b219
Added Put Mapping API to high-level Rest client (#27205)
catalin-ursachi Dec 18, 2017
c0385c1
Fixed comments
catalin-ursachi Dec 19, 2017
658eb7a
Review mark-ups
catalin-ursachi Jan 20, 2018
7cd96c6
Merge remote-tracking branch 'origin/master' into put-mapping
catalin-ursachi Jan 21, 2018
6a00310
Moved PutMappingResponseTests
catalin-ursachi Jan 21, 2018
b7a110e
Added Put Mapping docs
catalin-ursachi Jan 21, 2018
574d2fa
Merge remote-tracking branch 'Catalin/put-mapping' into put-mapping
catalin-ursachi Jan 21, 2018
9a7c924
Merge remote-tracking branch 'origin/master' into put-mapping
catalin-ursachi Jan 22, 2018
ca458e5
Review mark-ups
catalin-ursachi Jan 22, 2018
b9ec26b
Fixed import order
catalin-ursachi Jan 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexResponse;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
import org.elasticsearch.action.admin.indices.open.OpenIndexRequest;
import org.elasticsearch.action.admin.indices.open.OpenIndexResponse;

Expand Down Expand Up @@ -89,6 +91,29 @@ public void createAsync(CreateIndexRequest createIndexRequest, ActionListener<Cr
listener, Collections.emptySet(), headers);
}

/**
* Updates the mappings on an index using the Put Mapping API
* <p>
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-put-mapping.html">
* Put Mapping API on elastic.co</a>
*/
public PutMappingResponse putMapping(PutMappingRequest putMappingRequest, Header... headers) throws IOException {
return restHighLevelClient.performRequestAndParseEntity(putMappingRequest, Request::putMapping, PutMappingResponse::fromXContent,
Collections.emptySet(), headers);
}

/**
* Asynchronously updates the mappings on an index using the Put Mapping API
* <p>
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-put-mapping.html">
* Put Mapping API on elastic.co</a>
*/
public void putMappingAsync(PutMappingRequest putMappingRequest, ActionListener<PutMappingResponse> listener,
Header... headers) {
restHighLevelClient.performRequestAsyncAndParseEntity(putMappingRequest, Request::putMapping, PutMappingResponse::fromXContent,
listener, Collections.emptySet(), headers);
}

/**
* Opens an index using the Open Index API
* <p>
Expand Down
20 changes: 20 additions & 0 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.elasticsearch.action.admin.indices.close.CloseIndexRequest;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.admin.indices.open.OpenIndexRequest;
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.delete.DeleteRequest;
Expand Down Expand Up @@ -179,6 +180,21 @@ static Request createIndex(CreateIndexRequest createIndexRequest) throws IOExcep
return new Request(HttpPut.METHOD_NAME, endpoint, parameters.getParams(), entity);
}

static Request putMapping(PutMappingRequest putMappingRequest) throws IOException {
// The concreteIndex is an internal concept, not applicable to requests made over the REST API.
assert (putMappingRequest.getConcreteIndex() == null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given that a request is provided by users, this should rather be an exception that gets thrown. Also this assertion may make the user's application crash. Throw IllegalArgumentException instead?


String endpoint = endpoint(putMappingRequest.indices(), "_mapping", putMappingRequest.type());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to support putMappingRequest.getConcreteIndex() here? If so, how/when should it be used? Does it make sense to support in client-specific code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no that's an internal concept leaked to our external API. Maybe check in RestHighLevelClient that concreteIndex is not set, as it will be ignored.


Params parameters = Params.builder();
parameters.withTimeout(putMappingRequest.timeout());
parameters.withMasterTimeout(putMappingRequest.masterNodeTimeout());
parameters.withUpdateAllTypes(putMappingRequest.updateAllTypes());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this parameter has been removed from master, I think that you may see that once you merge master in. I need to remember to add it back though when backporting to 6.x.


HttpEntity entity = createEntity(putMappingRequest, REQUEST_BODY_CONTENT_TYPE);
return new Request(HttpPut.METHOD_NAME, endpoint, parameters.getParams(), entity);
}

static Request info() {
return new Request(HttpGet.METHOD_NAME, "/", Collections.emptyMap(), null);
}
Expand Down Expand Up @@ -455,6 +471,10 @@ static String endpoint(String[] indices, String[] types, String endpoint) {
return endpoint(String.join(",", indices), String.join(",", types), endpoint);
}

static String endpoint(String[] indices, String endpoint, String type) {
return endpoint(String.join(",", indices), endpoint, type);
}

/**
* Utility method to build request's endpoint.
*/
Expand Down
34 changes: 34 additions & 0 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexResponse;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
import org.elasticsearch.action.admin.indices.open.OpenIndexRequest;
import org.elasticsearch.action.admin.indices.open.OpenIndexResponse;
import org.elasticsearch.action.support.IndicesOptions;
Expand Down Expand Up @@ -108,6 +110,38 @@ public void testCreateIndex() throws IOException {
}
}

@SuppressWarnings("unchecked")
public void testPutMapping() throws IOException {
{
// Add mappings to index
String indexName = "mapping_index";
createIndex(indexName);

PutMappingRequest putMappingRequest = new PutMappingRequest(indexName);

putMappingRequest.type("type_name");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you remove this empty line and the one above please?

XContentBuilder mappingBuilder = JsonXContent.contentBuilder();
mappingBuilder.startObject().startObject("properties").startObject("field");
mappingBuilder.field("type", "text");
mappingBuilder.endObject().endObject().endObject();
putMappingRequest.source(mappingBuilder);

PutMappingResponse putMappingResponse =
execute(putMappingRequest, highLevelClient().indices()::putMapping, highLevelClient().indices()::putMappingAsync);
assertTrue(putMappingResponse.isAcknowledged());

Map<String, Object> indexMetaData = getIndexMetadata(indexName);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you remove this empty line?

Map<String, Object> mappingsData = (Map) indexMetaData.get("mappings");
Map<String, Object> typeData = (Map) mappingsData.get("type_name");
Map<String, Object> properties = (Map) typeData.get("properties");
Map<String, Object> field = (Map) properties.get("field");

assertEquals("text", field.get("type"));
}
}

public void testDeleteIndex() throws IOException {
{
// Delete index if exists
Expand Down
42 changes: 42 additions & 0 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.action.admin.indices.close.CloseIndexRequest;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.admin.indices.open.OpenIndexRequest;
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.bulk.BulkShardRequest;
Expand Down Expand Up @@ -325,6 +326,47 @@ public void testCreateIndex() throws IOException {
assertToXContentBody(createIndexRequest, request.getEntity());
}

public void testPutMapping() throws IOException {
PutMappingRequest putMappingRequest = new PutMappingRequest();

int numIndices = randomIntBetween(0, 5);
String[] indices = new String[numIndices];
for (int i = 0; i < numIndices; i++) {
indices[i] = "index-" + randomAlphaOfLengthBetween(2, 5);
}
putMappingRequest.indices(indices);

String type = randomAlphaOfLengthBetween(3, 10);
putMappingRequest.type(type);

Map<String, String> expectedParams = new HashMap<>();

setRandomTimeout(putMappingRequest::timeout, AcknowledgedRequest.DEFAULT_ACK_TIMEOUT, expectedParams);
setRandomMasterTimeout(putMappingRequest, expectedParams);

if (randomBoolean()) {
boolean updateAllTypes = randomBoolean();
putMappingRequest.updateAllTypes(updateAllTypes);
if (updateAllTypes) {
expectedParams.put("update_all_types", Boolean.TRUE.toString());
}
}

Request request = Request.putMapping(putMappingRequest);
StringJoiner endpoint = new StringJoiner("/", "/", "");
String index = String.join(",", indices);
if (Strings.hasLength(index)) {
endpoint.add(index);
}
endpoint.add("_mapping");
endpoint.add(type);
assertEquals(endpoint.toString(), request.getEndpoint());

assertEquals(expectedParams, request.getParameters());
assertEquals("PUT", request.getMethod());
assertToXContentBody(putMappingRequest, request.getEntity());
}

public void testDeleteIndex() {
String[] indices = randomIndicesNames(0, 5);
DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(indices);
Expand Down
100 changes: 91 additions & 9 deletions ...el/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexResponse;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
import org.elasticsearch.action.admin.indices.open.OpenIndexRequest;
import org.elasticsearch.action.admin.indices.open.OpenIndexResponse;
import org.elasticsearch.action.support.ActiveShardCount;
Expand Down Expand Up @@ -157,15 +159,15 @@ public void testCreateIndex() throws IOException {

// tag::create-index-request-mappings
request.mapping("tweet", // <1>
" {\n" +
" \"tweet\": {\n" +
" \"properties\": {\n" +
" \"message\": {\n" +
" \"type\": \"text\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }", // <2>
"{\n" +
" \"tweet\": {\n" +
" \"properties\": {\n" +
" \"message\": {\n" +
" \"type\": \"text\"\n" +
" }\n" +
" }\n" +
" }\n" +
"}", // <2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this required? Maybe you can revert this small part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IDE indented the lines; will revert.

Would you like me to add the two extra spaces per line within the string back in? They seemed superfluous.

XContentType.JSON);
// end::create-index-request-mappings

Expand Down Expand Up @@ -228,6 +230,86 @@ public void onFailure(Exception e) {
}
}

public void testPutMapping() throws IOException {
RestHighLevelClient client = highLevelClient();

{
CreateIndexResponse createIndexResponse = client.indices().create(new CreateIndexRequest("twitter"));
assertTrue(createIndexResponse.isAcknowledged());
}

{
// tag::put-mapping-request
PutMappingRequest request = new PutMappingRequest("twitter"); // <1>
request.type("tweet"); // <2>
// end::put-mapping-request

// tag::put-mapping-request-source
request.source(
"{\n" +
" \"tweet\": {\n" +
" \"properties\": {\n" +
" \"message\": {\n" +
" \"type\": \"text\"\n" +
" }\n" +
" }\n" +
" }\n" +
"}", // <1>
XContentType.JSON);
// end::put-mapping-request-source

// tag::put-mapping-request-timeout
request.timeout(TimeValue.timeValueMinutes(2)); // <1>
request.timeout("2m"); // <2>
// end::put-mapping-request-timeout
// tag::put-mapping-request-masterTimeout
request.masterNodeTimeout(TimeValue.timeValueMinutes(1)); // <1>
request.masterNodeTimeout("1m"); // <2>
// end::put-mapping-request-masterTimeout

// tag::put-mapping-execute
PutMappingResponse putMappingResponse = client.indices().putMapping(request);
// end::put-mapping-execute

// tag::put-mapping-response
boolean acknowledged = putMappingResponse.isAcknowledged(); // <1>
// end::put-mapping-response
assertTrue(acknowledged);
}
}

public void testPutMappingAsync() throws Exception {
final RestHighLevelClient client = highLevelClient();

{
CreateIndexResponse createIndexResponse = client.indices().create(new CreateIndexRequest("twitter"));
assertTrue(createIndexResponse.isAcknowledged());
}

{
PutMappingRequest request = new PutMappingRequest("twitter").type("tweet");
// tag::put-mapping-execute-async
client.indices().putMappingAsync(request, new ActionListener<PutMappingResponse>() {
@Override
public void onResponse(PutMappingResponse putMappingResponse) {
// <1>
}

@Override
public void onFailure(Exception e) {
// <2>
}
});
// end::put-mapping-execute-async

assertBusy(() -> {
// TODO Use Indices Exist API instead once it exists
Response response = client.getLowLevelClient().performRequest("HEAD", "twitter");
assertTrue(RestStatus.OK.getStatus() == response.getStatusLine().getStatusCode());
});
}
}

public void testOpenIndex() throws IOException {
RestHighLevelClient client = highLevelClient();

Expand Down
2 changes: 2 additions & 0 deletions docs/java-rest/high-level/apis/index.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ include::open_index.asciidoc[]

include::close_index.asciidoc[]

include::putmapping.asciidoc[]

include::_index.asciidoc[]

include::get.asciidoc[]
Expand Down
71 changes: 71 additions & 0 deletions docs/java-rest/high-level/apis/putmapping.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
[[java-rest-high-put-mapping]]
=== Put Mapping API

[[java-rest-high-put-mapping-request]]
==== Put Mapping Request

A `PutMappingRequest` requires an `index` argument, and a type:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-mapping-request]
--------------------------------------------------
<1> The index to add the mapping to
<2> The type to create (or update)

==== Mapping source
A description of the fields to create on the mapping; if not defined, the mapping will default to empty.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we list this under the optional arguments? After all it is optional?

Copy link
Contributor Author

@catalin-ursachi catalin-ursachi Jan 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Technically it's optional; but a PutMapping request without a source just adds a new type; which, in the ES 6.x+ world, is basically meaningless.


["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-mapping-request-source]
--------------------------------------------------
<1> The mapping source
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we cover the different way to provide the source like we did in other docs pages (e.g. search API)? I see that we didn't do it for the create index API, but maybe we should have. Shall we have a common page on how to provide a source and link to it? What do you think? Maybe we could also do it as a followup so this PR doesn't get too big.

Copy link
Contributor Author

@catalin-ursachi catalin-ursachi Jan 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does make sense; but it does rather seem like there could be some more user-friendly mapping-building tools, abstracting out the boilerplate. E.g. the ones in Elastic4s:

    createIndex("artists").mappings(
      mapping("modern").fields(
        textField("name")
      )
    )

(https://github.com/sksamuel/elastic4s)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But anyway; as things are now, I think a separate doc makes sense, so as not to clutter the Create Index and Put Mapping pages too much?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we should do better, but for now we are only porting what the transport client supports to the new client. I think that having a separate page would be good so we don't repeat the same content in different pages, I assume that the content would be the same, it can probably already be extracted from the search docs page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool; I am tempted to suggest doing it as a follow-up PR, then, since I'm not sure I've got the time to do it just now. If you think it's a blocker for this one, though, then that's fair enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, fine as a follow-up PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@catalin-ursachi let me know if you want to take this, otherwise I can take care of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By all means feel free to take it; otherwise I'll pick it up when I next set aside some time for OS work, but I can't promise when that is. 🙂


==== Optional arguments
The following arguments can optionally be provided:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-mapping-request-timeout]
--------------------------------------------------
<1> Timeout to wait for the all the nodes to acknowledge the index creation as a `TimeValue`
<2> Timeout to wait for the all the nodes to acknowledge the index creation as a `String`

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-mapping-request-masterTimeout]
--------------------------------------------------
<1> Timeout to connect to the master node as a `TimeValue`
<2> Timeout to connect to the master node as a `String`

[[java-rest-high-put-mapping-sync]]
==== Synchronous Execution

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-mapping-execute]
--------------------------------------------------

[[java-rest-high-put-mapping-async]]
==== Asynchronous Execution

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-mapping-execute-async]
--------------------------------------------------
<1> Called when the execution is successfully completed. The response is
provided as an argument
<2> Called in case of failure. The raised exception is provided as an argument

[[java-rest-high-put-mapping-response]]
==== Put Mapping Response

The returned `PutMappingResponse` allows to retrieve information about the executed
operation as follows:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-mapping-response]
--------------------------------------------------
<1> Indicates whether all of the nodes have acknowledged the request
1 change: 1 addition & 0 deletions docs/java-rest/high-level/supported-apis.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Indices APIs::
* <<java-rest-high-delete-index>>
* <<java-rest-high-open-index>>
* <<java-rest-high-close-index>>
* <<java-rest-high-put-mapping>>

Single document APIs::
* <<java-rest-high-document-index>>
Expand Down
Loading