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

Add Get Aliases API to the high-level REST client #28799

Merged
merged 27 commits into from
Jun 12, 2018
Merged

Add Get Aliases API to the high-level REST client #28799

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
6c01eeb
Add Get Aliases API to the high-level RESR client
olcbean Feb 22, 2018
1571cb3
next iteration : still work needed in GetAliasesResponseTests
olcbean Mar 19, 2018
a33ea0f
addressing some comments
olcbean Apr 10, 2018
a919d73
Merge remote-tracking branch 'origin/master' into hlrc_get_alias
olcbean Apr 13, 2018
baed505
chmod
olcbean Apr 13, 2018
f74d0b5
Merge branch 'master' into hlrc_get_alias
javanna May 11, 2018
7720b71
addressing reviewers comments
olcbean May 18, 2018
7238098
addressing reviewers comments
olcbean May 22, 2018
e2ffdab
insert random data into AliasMetaData
olcbean May 23, 2018
9a2079f
adding serialization bwc tests
olcbean May 25, 2018
df3b2fb
adding a serialization test to verify that a response serialized by 6…
olcbean May 28, 2018
0e3ad61
clean up
olcbean May 29, 2018
519992b
clean up
olcbean May 29, 2018
11811ef
Merge branch 'master' into hlrc_get_alias
javanna May 30, 2018
474238b
Merge branch 'master' into hlrc_get_alias
javanna May 30, 2018
a59a026
Merge branch 'master' into hlrc_get_alias
javanna May 31, 2018
b210f49
Merge branch 'master' into hlrc_get_alias
javanna May 31, 2018
5b3096b
Merge branch 'master' into hlrc_get_alias
javanna Jun 5, 2018
8660d43
fix bw comp issues
javanna Jun 5, 2018
f21b3ca
remove unclear if
javanna Jun 5, 2018
fa9d4e1
introduce client specific GetAliasesResponse
javanna Jun 5, 2018
939ae4d
Merge branch 'master' into hlrc_get_alias
javanna Jun 6, 2018
035d823
address review comments
javanna Jun 6, 2018
be1daf7
Merge branch 'master' into hlrc_get_alias
javanna Jun 6, 2018
5da9ed6
fix javadocs
javanna Jun 7, 2018
5bcc1ce
address review comments
javanna Jun 11, 2018
2e2217a
Merge branch 'master' into hlrc_get_alias
javanna Jun 11, 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
26 changes: 26 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 @@ -24,6 +24,7 @@
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse;
import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheResponse;
import org.elasticsearch.action.admin.indices.close.CloseIndexRequest;
Expand All @@ -47,11 +48,13 @@
import org.elasticsearch.action.admin.indices.rollover.RolloverResponse;
import org.elasticsearch.action.admin.indices.shrink.ResizeRequest;
import org.elasticsearch.action.admin.indices.shrink.ResizeResponse;
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;
import java.util.Collections;

import static java.util.Collections.emptySet;
import static java.util.Collections.singleton;

/**
* A wrapper for the {@link RestHighLevelClient} that provides methods for accessing the Indices API.
Expand Down Expand Up @@ -406,4 +409,27 @@ public void rolloverAsync(RolloverRequest rolloverRequest, ActionListener<Rollov
restHighLevelClient.performRequestAsyncAndParseEntity(rolloverRequest, Request::rollover, RolloverResponse::fromXContent,
listener, emptySet(), headers);
}

/**
* Gets one or more aliases using the Get Index Aliases API
* <p>
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-aliases.html"> Indices Aliases API on
* elastic.co</a>
*/
public GetAliasesResponse getAlias(GetAliasesRequest getAliasesRequest, Header... headers) throws IOException {
return restHighLevelClient.performRequestAndParseEntity(getAliasesRequest, Request::getAlias, GetAliasesResponse::fromXContent,
singleton(RestStatus.NOT_FOUND.getStatus()), headers);
}

/**
* Asynchronously gets one or more aliases using the Get Index Aliases API
* <p>
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-aliases.html"> Indices Aliases API on
* elastic.co</a>
*/
public void getAliasAsync(GetAliasesRequest getAliasesRequest, ActionListener<GetAliasesResponse> listener, Header... headers) {
restHighLevelClient.performRequestAsyncAndParseEntity(getAliasesRequest, Request::getAlias, GetAliasesResponse::fromXContent,
listener, singleton(RestStatus.NOT_FOUND.getStatus()), headers);
}

}
12 changes: 12 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 @@ -64,6 +64,7 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ToXContent;
Expand Down Expand Up @@ -607,6 +608,17 @@ static Request indicesExist(GetIndexRequest request) {
return new Request(HttpHead.METHOD_NAME, endpoint, params.getParams(), null);
}

static Request getAlias(GetAliasesRequest getAliasesRequest) {
Params params = Params.builder();
params.withIndicesOptions(getAliasesRequest.indicesOptions());
params.withLocal(getAliasesRequest.local());

String[] indices = getAliasesRequest.indices() == null ? Strings.EMPTY_ARRAY : getAliasesRequest.indices();
String[] aliases = getAliasesRequest.aliases() == null ? Strings.EMPTY_ARRAY : getAliasesRequest.aliases();
String endpoint = endpoint(indices, "_alias", aliases);
return new Request(HttpGet.METHOD_NAME, endpoint, params.getParams(), null);
}

private static HttpEntity createEntity(ToXContent toXContent, XContentType xContentType) throws IOException {
BytesRef source = XContentHelper.toXContent(toXContent, xContentType, false).toBytesRef();
return new ByteArrayEntity(source.bytes, source.offset, source.length, createContentType(xContentType));
Expand Down
16 changes: 8 additions & 8 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,10 @@ protected final <Req extends ActionRequest, Resp> Resp performRequest(Req reques
try {
return responseConverter.apply(e.getResponse());
} catch (Exception innerException) {
//the exception is ignored as we now try to parse the response as an error.
//this covers cases like get where 404 can either be a valid document not found response,
//or an error for which parsing is completely different. We try to consider the 404 response as a valid one
//first. If parsing of the response breaks, we fall back to parsing it as an error.
// the exception is ignored as we now try to parse the response as an error.
// this covers cases like get where 404 can either be a valid document not found response,
// or an error for which parsing is completely different. We try to consider the 404 response as a valid one
// first. If parsing of the response breaks, we fall back to parsing it as an error.
throw parseResponseException(e);
}
}
Expand Down Expand Up @@ -593,10 +593,10 @@ public void onFailure(Exception exception) {
try {
actionListener.onResponse(responseConverter.apply(response));
} catch (Exception innerException) {
//the exception is ignored as we now try to parse the response as an error.
//this covers cases like get where 404 can either be a valid document not found response,
//or an error for which parsing is completely different. We try to consider the 404 response as a valid one
//first. If parsing of the response breaks, we fall back to parsing it as an error.
// the exception is ignored as we now try to parse the response as an error.
// this covers cases like get where 404 can either be a valid document not found response,
// or an error for which parsing is completely different. We try to consider the 404 response as a valid one
// first. If parsing of the response breaks, we fall back to parsing it as an error.
actionListener.onFailure(parseResponseException(responseException));
}
} else {
Expand Down
169 changes: 169 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 @@ -28,6 +28,7 @@
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse;
import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheResponse;
import org.elasticsearch.action.admin.indices.close.CloseIndexRequest;
Expand Down Expand Up @@ -72,6 +73,8 @@
import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;

public class IndicesClientIT extends ESRestHighLevelClientTestCase {

Expand Down Expand Up @@ -609,4 +612,170 @@ public void testRollover() throws IOException {
assertEquals("test_new", rolloverResponse.getNewIndex());
}
}

public void testGetAlias() throws IOException {
{
createIndex("index1", Settings.EMPTY);
client().performRequest(HttpPut.METHOD_NAME, "/index1/_alias/alias1");

createIndex("index2", Settings.EMPTY);
client().performRequest(HttpPut.METHOD_NAME, "/index2/_alias/alias2");

createIndex("index3", Settings.EMPTY);
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().aliases("alias1");
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);

assertThat(getAliasesResponse.getAliases().size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).alias(), equalTo("alias1"));
assertThat(getAliasesResponse.getAliases().get("index1").get(0).getFilter(), nullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).getIndexRouting(), nullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).getSearchRouting(), nullValue());
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().aliases("alias*");
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);

assertThat(getAliasesResponse.getAliases().size(), equalTo(2));
assertThat(getAliasesResponse.getAliases().get("index1").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).alias(), equalTo("alias1"));
assertThat(getAliasesResponse.getAliases().get("index2").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index2").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index2").get(0).alias(), equalTo("alias2"));
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().aliases("_all");
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);

assertThat(getAliasesResponse.getAliases().size(), equalTo(2));
assertThat(getAliasesResponse.getAliases().get("index1").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).alias(), equalTo("alias1"));
assertThat(getAliasesResponse.getAliases().get("index2").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index2").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index2").get(0).alias(), equalTo("alias2"));
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().aliases("*");
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);

assertThat(getAliasesResponse.getAliases().size(), equalTo(2));
assertThat(getAliasesResponse.getAliases().get("index1").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).alias(), equalTo("alias1"));
assertThat(getAliasesResponse.getAliases().get("index2").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index2").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index2").get(0).alias(), equalTo("alias2"));
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().indices("_all");
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);

assertThat(getAliasesResponse.getAliases().size(), equalTo(3));
assertThat(getAliasesResponse.getAliases().get("index1").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).alias(), equalTo("alias1"));
assertThat(getAliasesResponse.getAliases().get("index2").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index2").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index2").get(0).alias(), equalTo("alias2"));
assertThat(getAliasesResponse.getAliases().get("index3").size(), equalTo(0));
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().indices("ind*");
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);

assertThat(getAliasesResponse.getAliases().size(), equalTo(3));
assertThat(getAliasesResponse.getAliases().get("index1").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).alias(), equalTo("alias1"));
assertThat(getAliasesResponse.getAliases().get("index2").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index2").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index2").get(0).alias(), equalTo("alias2"));
assertThat(getAliasesResponse.getAliases().get("index3").size(), equalTo(0));
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest();
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);

assertThat(getAliasesResponse.getAliases().size(), equalTo(3));
assertThat(getAliasesResponse.getAliases().get("index1").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index1").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index1").get(0).alias(), equalTo("alias1"));
assertThat(getAliasesResponse.getAliases().get("index2").size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get("index2").get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get("index2").get(0).alias(), equalTo("alias2"));
assertThat(getAliasesResponse.getAliases().get("index3").size(), equalTo(0));
}
}

public void testGetAliasesNonExistentIndexOrAlias() 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.

I think it is worth leaving a comment that we are so thorough here because it is the only way we can check that we haven't slid out of sync with the server because the server renders the xcontent in a spot that is difficult for us to access in a unit test.


String alias = "alias";
String index = "index";
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().indices(index);
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> execute(getAliasesRequest, highLevelClient().indices()::getAlias, highLevelClient().indices()::getAliasAsync));
assertThat(exception.status(), equalTo(RestStatus.NOT_FOUND));
assertThat(exception.getMessage(), equalTo("Elasticsearch exception [type=index_not_found_exception, reason=no such index]"));
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest(alias);
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> execute(getAliasesRequest, highLevelClient().indices()::getAlias, highLevelClient().indices()::getAliasAsync));
assertThat(exception.status(), equalTo(RestStatus.NOT_FOUND));
assertThat(exception.getMessage(), equalTo("Elasticsearch exception [type=exception, reason=alias [" + alias + "] missing]"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am somehow stuck with this response in case of an error. I do not see an option which "makes sense".

A request with non existent alias results in :

{
  "error": "alias [<alias>] missing",
  "status": 404
}

For now I decided to map it to an exception. Basically if there are no aliases found then throw. If at least one alias is found, a response will be returned and its status must be checked ( to verify if all aliases have been found : OK , or if this is a "partial" response : NOT_FOUND)

Another option is to throw only if any of the indices do not exist. And for the aliases always to return a response ( even if none of the specified aliases exist ).

Yet another option is to never throw... ( which is not is sync with the rest of the apis ... )

Ideas how to proceed ?

Copy link
Member

Choose a reason for hiding this comment

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

this is a weird response. I think there isn't a perfect solution. I initially was going for your solution, only throw an error when no aliases are returned. But that way that there is a 404 that's ignored and a 404 that leads to an exception being thrown, which is confusing. Also, it makes the code messy as we end up having to ignore 404 in the client, yet we need to throw in some specific case exception in fromXContent. I would then go for never throwing. It is always possible to retrieve the returned status and error message, so there is no missing info. And I will open an issue to discuss how we can fix this response as a follow-up.

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 get alias response is really confusing ;)

To be honest, I went with the least disruption for the users I could come up with : parse the response if there are any aliases returned ( even if the response contains only 'partial' results and use the additional info such as error message and status if need be ). Confusing and hard to maintain on the server side, but easier for the users ;)

If 404 is ignored and no exception is thrown, then both an error message and an ES exception will need to be stored in the response object ;) and the users will need to first check the status and if it is 404, then check if there is an ES exception or a string exception with partial results.

@javanna shall I give this approach a try or should we wait for some directions from #30536 ?

Copy link
Member

Choose a reason for hiding this comment

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

no let's not wait, I would get this one in, for now let's never throw exception, users can read errors if they wish.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javanna one more thing: if ES exception is added to the response, should it be serialized on the transport layer ? Maybe it would be better not to serialize it on the transport layer, but then the response tests could get tricky and maybe they will need to be split ( something similar to what has been done in #28892 )

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure. At the moment we only have a string and a status, are you going for recreating an exception based on that info?

}
createIndex(index, Settings.EMPTY);
client().performRequest(HttpPut.METHOD_NAME, index + "/_alias/" + alias);
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().indices(index, "non_existent_index");
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> execute(getAliasesRequest, highLevelClient().indices()::getAlias, highLevelClient().indices()::getAliasAsync));
assertThat(exception.status(), equalTo(RestStatus.NOT_FOUND));
assertThat(exception.getMessage(), equalTo("Elasticsearch exception [type=index_not_found_exception, reason=no such index]"));
}
{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().indices(index, "non_existent_index").aliases(alias);
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> execute(getAliasesRequest, highLevelClient().indices()::getAlias, highLevelClient().indices()::getAliasAsync));
assertThat(exception.status(), equalTo(RestStatus.NOT_FOUND));
assertThat(exception.getMessage(), equalTo("Elasticsearch exception [type=index_not_found_exception, reason=no such index]"));
}
Copy link
Member

Choose a reason for hiding this comment

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

I noticed that when a wildcard expressions is provided for the indices, and no indices match, we return 200 and an empty json object. Can we test that too?

curl 'localhost:9200/nothing*/_alias?pretty'
{ }

{
GetAliasesRequest getAliasesRequest = new GetAliasesRequest().indices(index).aliases(alias, "non_existent_alias");
GetAliasesResponse getAliasesResponse = execute(getAliasesRequest, highLevelClient().indices()::getAlias,
highLevelClient().indices()::getAliasAsync);
assertThat(getAliasesResponse.status(), equalTo(RestStatus.NOT_FOUND));

assertThat(getAliasesResponse.getAliases().size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get(index).size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get(index).get(0), notNullValue());
assertThat(getAliasesResponse.getAliases().get(index).get(0).alias(), equalTo(alias));

/*
Copy link
Member

Choose a reason for hiding this comment

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

Leftover or intentional? If intentional is probably fine but it is worth formatting and leaving a little explanatory comment.

Copy link
Member

Choose a reason for hiding this comment

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

intentional, I will add a comment

{
"error": "alias [something] missing",
"status": 404,
"index": {
"aliases": {
"alias": {}
}
}
}
*/
}
}

}
Loading