Skip to content
Merged
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.client;

import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.ProtocolVersion;
import org.apache.http.RequestLine;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.entity.ContentType;
import org.apache.http.message.BasicHttpResponse;
import org.apache.http.message.BasicRequestLine;
import org.apache.http.message.BasicStatusLine;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.ValidateActions;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;

import java.io.IOException;

import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static org.elasticsearch.client.ESRestHighLevelClientTestCase.execute;
import static org.elasticsearch.client.Request.REQUEST_BODY_CONTENT_TYPE;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyMapOf;
import static org.mockito.Matchers.anyObject;
import static org.mockito.Matchers.anyVararg;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;

/**
* Test and demonstrates how {@link RestHighLevelClient} can be extended to support custom requests and responses.
Copy link
Member

Choose a reason for hiding this comment

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

add "against custom endpoints" at the end of the sentence?

*/
public class CustomRestHighLevelClientTests extends ESTestCase {

private static final String ENDPOINT = "/_custom";

private CustomRestClient restHighLevelClient;

@Before
public void iniClients() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

initClients

if (restHighLevelClient == null) {
final RestClient restClient = mock(RestClient.class);
restHighLevelClient = new CustomRestClient(restClient);

doAnswer(mock -> performRequest((HttpEntity) mock.getArguments()[3]))
.when(restClient)
.performRequest(eq(HttpGet.METHOD_NAME), eq(ENDPOINT), anyMapOf(String.class, String.class), anyObject(), anyVararg());

doAnswer(mock -> performRequestAsync((HttpEntity) mock.getArguments()[3], (ResponseListener) mock.getArguments()[4]))
.when(restClient)
.performRequestAsync(eq(HttpGet.METHOD_NAME), eq(ENDPOINT), anyMapOf(String.class, String.class),
any(HttpEntity.class), any(ResponseListener.class), anyVararg());
}
}

public void testCustomRequest() throws IOException {
final int length = randomIntBetween(1, 10);

CustomRequest customRequest = new CustomRequest();
customRequest.setValue(randomAlphaOfLength(length));

CustomResponse customResponse = execute(customRequest, restHighLevelClient::custom, restHighLevelClient::customAsync);
assertEquals(length, customResponse.getLength());
assertEquals(expectedStatus(length), customResponse.status());
}

private Void performRequestAsync(HttpEntity httpEntity, ResponseListener responseListener) {
Copy link
Member

Choose a reason for hiding this comment

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

why Void instead of void?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because Mockito's doAnswer() method needs a type to be able to stub a method that returns void (ie RestClient's performRequestAsync method).

try {
responseListener.onSuccess(performRequest(httpEntity));
} catch (IOException e) {
responseListener.onFailure(e);
}
return null;
}

private Response performRequest(HttpEntity httpEntity) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

why do we need these two methods? can we use the internal RestClient instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why do you mean by using the internal RestClient? The test uses a mocked RestClient and executes these methods when the RestClient's performRequest and performRequestAsync methods are called with parameters that match the Matchers configured in initClient(). I might be missing something, but I think this test mocks the right RestClient object ?

Copy link
Member

Choose a reason for hiding this comment

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

right, I had totally misunderstood how the test works

try (XContentParser parser = createParser(REQUEST_BODY_CONTENT_TYPE.xContent(), httpEntity.getContent())) {
CustomRequest request = CustomRequest.fromXContent(parser);

int length = request.getValue() != null ? request.getValue().length() : -1;
CustomResponse response = new CustomResponse(length);

ProtocolVersion protocol = new ProtocolVersion("HTTP", 1, 1);
RestStatus status = response.status();
HttpResponse httpResponse = new BasicHttpResponse(new BasicStatusLine(protocol, status.getStatus(), status.name()));

BytesRef bytesRef = XContentHelper.toXContent(response, XContentType.JSON, false).toBytesRef();
httpResponse.setEntity(new ByteArrayEntity(bytesRef.bytes, ContentType.APPLICATION_JSON));

RequestLine requestLine = new BasicRequestLine(HttpGet.METHOD_NAME, ENDPOINT, protocol);
return new Response(requestLine, new HttpHost("localhost", 9200), httpResponse);
}
}

/**
* A custom high level client that provides methods to execute a custom request and get its associate response back.
*/
static class CustomRestClient extends RestHighLevelClient {

private CustomRestClient(RestClient restClient) {
super(restClient);
}

public CustomResponse custom(CustomRequest customRequest, Header... headers) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

shall we also have two additional methods that use performRequestAsyncAndParseEntity and performRequestAndParseEntity (which should be made protected for that)?

return performRequest(customRequest, this::toRequest, this::toResponse, emptySet(), headers);
Copy link
Member

Choose a reason for hiding this comment

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

I think that we need to make this method and performRequestAsync protected rather than package private. Can you add for this and the other needed methods a test like this one: https://github.com/elastic/elasticsearch/blob/master/client/rest/src/test/java/org/elasticsearch/client/HeapBufferedAsyncResponseConsumerTests.java#L121 ?

}

public void customAsync(CustomRequest customRequest, ActionListener<CustomResponse> listener, Header... headers) {
performRequestAsync(customRequest, this::toRequest, this::toResponse, listener, emptySet(), headers);
Copy link
Member

Choose a reason for hiding this comment

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

I think that what confuses me here is that we call performRequest and performRequestAsync here, why do we mock the rest client then? Wouldn't it be better to test that RestHighLevelClient subclasses can use performRequestAndParseEntity and performRequestAsyncAndParseEntity (which are private at the moment)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that what confuses me here is that we call performRequest and performRequestAsync here, why do we mock the rest client then?

I agree, this is confusing. I you noticed it calls the performRequest method from the RestHighLevelClient and not the method from the test. I renamed the test method so that it's not confusing.

Wouldn't it be better to test that RestHighLevelClient subclasses can use performRequestAndParseEntity and performRequestAsyncAndParseEntity (which are private at the moment)

I don't know, it really depends of what the client want to do with the HTTP response. The performRequest/performRequestAsync are nice because we get back the Response from the low level rest client and so we can extract headers and still parse the entity with the parseEntity method. On the other side, exposing performRequestAndParseEntity and performRequestAsyncAndParseEntity would be useful too, so maybe both of them should be protected.

}

Request toRequest(CustomRequest customRequest) throws IOException {
BytesRef source = XContentHelper.toXContent(customRequest, REQUEST_BODY_CONTENT_TYPE, false).toBytesRef();
ContentType contentType = ContentType.create(REQUEST_BODY_CONTENT_TYPE.mediaType());
HttpEntity entity = new ByteArrayEntity(source.bytes, source.offset, source.length, contentType);
return new Request(HttpGet.METHOD_NAME, ENDPOINT, emptyMap(), entity);
}

CustomResponse toResponse(Response response) throws IOException {
return parseEntity(response.getEntity(), CustomResponse::fromXContent);
}
}

static class CustomRequest extends ActionRequest implements ToXContentObject {
Copy link
Member

Choose a reason for hiding this comment

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

maybe we could trim down the test and just reuse existing requests? would it make the test less useful? I think the important bit is that you can add the methods to the custom client class, and that those methods can make use of the existing infra.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to keep the custom request and custom response because it echoes the custom/customAsync methods. But I can trim down the test by not printing out xcontent request and response body which are not the point here. I'll try that and you'll tell me if you still want to reuse existing requests (I'm fine with reusing MainRequest/MainResponse for example)


private String value;

CustomRequest() {
}

public String getValue() {
return value;
}

public void setValue(String value) {
this.value = value;
}

@Override
public ActionRequestValidationException validate() {
if (Strings.hasLength(value) == false) {
return ValidateActions.addValidationError("value is missing", null);
}
return null;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.startObject().field("value", value).endObject();
}

private static final ObjectParser<CustomRequest, Void> PARSER = new ObjectParser<>("custom_request", CustomRequest::new);
static {
PARSER.declareString(CustomRequest::setValue, new ParseField("value"));
}

static CustomRequest fromXContent(XContentParser parser) throws IOException {
return PARSER.parse(parser, null);
}
}

static class CustomResponse extends ActionResponse implements StatusToXContentObject {

private final int length;

CustomResponse(int length) {
this.length = length;
}

public int getLength() {
return length;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.startObject().field("length", length).endObject();
}

@Override
public RestStatus status() {
return expectedStatus(getLength());
}

private static final ConstructingObjectParser<CustomResponse, Void> PARSER =
new ConstructingObjectParser<>("custom_response", args -> new CustomResponse((int) args[0]));
static {
PARSER.declareInt(ConstructingObjectParser.constructorArg(), new ParseField("length"));
}

static CustomResponse fromXContent(XContentParser parser) throws IOException {
return PARSER.parse(parser, null);
}
}

private static RestStatus expectedStatus(int length) {
return length > 5 ? RestStatus.OK : RestStatus.BAD_REQUEST;
}
}