Skip to content

Check that client methods match API defined in the REST spec #31825

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

Merged
merged 11 commits into from
Jul 17, 2018
Merged

Check that client methods match API defined in the REST spec #31825

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5ec0f4c
Check that exposed methods match API as defined in the REST spec
javanna Jul 5, 2018
33d86a6
remove needless dependency
javanna Jul 5, 2018
bfc28a2
Merge branch 'master' into test/hl_client_spec
javanna Jul 6, 2018
4e035aa
address part of the review comments
javanna Jul 6, 2018
16ec7a2
deprecate old methods
javanna Jul 6, 2018
fba6b12
remove empty lines and unused import
javanna Jul 6, 2018
0dee911
Merge branch 'master' into test/hl_client_spec
javanna Jul 16, 2018
e0d6635
revert to copying spec files, adapt to changes upstream
javanna Jul 16, 2018
7086d08
add todo on xpack api
javanna Jul 16, 2018
72e9fa4
revert some of the changes
javanna Jul 16, 2018
dc3c55e
add comments
javanna Jul 16, 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
3 changes: 1 addition & 2 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import org.elasticsearch.gradle.VersionProperties
import org.gradle.api.DefaultTask
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.Transformer
import org.gradle.api.execution.TaskExecutionAdapter
import org.gradle.api.internal.tasks.options.Option
import org.gradle.api.provider.Property
Expand Down Expand Up @@ -217,7 +216,7 @@ public class RestIntegTestTask extends DefaultTask {
* @param project The project to add the copy task to
* @param includePackagedTests true if the packaged tests should be copied, false otherwise
*/
private static Task createCopyRestSpecTask(Project project, Provider<Boolean> includePackagedTests) {
static Task createCopyRestSpecTask(Project project, Provider<Boolean> includePackagedTests) {
project.configurations {
restSpec
}
Expand Down
8 changes: 6 additions & 2 deletions client/rest-high-level/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
*/

import org.elasticsearch.gradle.precommit.PrecommitTasks
import org.gradle.api.XmlProvider
import org.gradle.api.publish.maven.MavenPublication
import org.elasticsearch.gradle.test.RestIntegTestTask
import org.gradle.api.internal.provider.Providers

buildscript {
repositories {
Expand All @@ -41,6 +41,9 @@ apply plugin: 'com.github.johnrengelman.shadow'
group = 'org.elasticsearch.client'
archivesBaseName = 'elasticsearch-rest-high-level-client'

Task copyRestSpec = RestIntegTestTask.createCopyRestSpecTask(project, Providers.FALSE)
test.dependsOn(copyRestSpec)
Copy link
Member Author

@javanna javanna Jul 5, 2018

Choose a reason for hiding this comment

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

these build changes are temporary, I just hacked to make the new test work. The new test needs the spec in its classpath, yet it is not an integ test. Suggestions are welcome on how to improve this and make this cleaner.


publishing {
publications {
nebula(MavenPublication) {
Expand Down Expand Up @@ -102,6 +105,7 @@ dependencies {
testCompile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
testCompile "junit:junit:${versions.junit}"
testCompile "org.hamcrest:hamcrest-all:${versions.hamcrest}"
testCompile "org.elasticsearch:rest-api-spec:${version}"
Copy link
Member Author

@javanna javanna Jul 6, 2018

Choose a reason for hiding this comment

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

funnily, this makes it work from intellij, as stuff is found on filesystem, extracted from jars. not sure what this does in Eclipse.

}

dependencyLicenses {
Expand Down
35 changes: 32 additions & 3 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 @@ -174,7 +174,7 @@ public void putMappingAsync(PutMappingRequest putMappingRequest, RequestOptions
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public GetMappingsResponse getMappings(GetMappingsRequest getMappingsRequest, RequestOptions options) throws IOException {
public GetMappingsResponse getMapping(GetMappingsRequest getMappingsRequest, RequestOptions options) throws IOException {
Copy link
Member Author

@javanna javanna Jul 5, 2018

Choose a reason for hiding this comment

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

All these changes are breaking. I guess we should deprecate the old methods in 6.x and have the test ignore those? It is pretty annoying that we just released some of these (some haven't been released yet so we are lucky there) and we already need to rename them...

Copy link
Member

@nik9000 nik9000 Jul 5, 2018

Choose a reason for hiding this comment

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

I agree we should deprecate the old names and remove in 7.0. Certainly annoying but we really are trying to be better about breaking the client.

return restHighLevelClient.performRequestAndParseEntity(getMappingsRequest, RequestConverters::getMappings, options,
GetMappingsResponse::fromXContent, emptySet());
}
Expand All @@ -187,8 +187,8 @@ public GetMappingsResponse getMappings(GetMappingsRequest getMappingsRequest, Re
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public void getMappingsAsync(GetMappingsRequest getMappingsRequest, RequestOptions options,
ActionListener<GetMappingsResponse> listener) {
public void getMappingAsync(GetMappingsRequest getMappingsRequest, RequestOptions options,
ActionListener<GetMappingsResponse> listener) {
restHighLevelClient.performRequestAsyncAndParseEntity(getMappingsRequest, RequestConverters::getMappings, options,
GetMappingsResponse::fromXContent, listener, emptySet());
}
Expand Down Expand Up @@ -474,8 +474,23 @@ public void getAsync(GetIndexRequest getIndexRequest, RequestOptions options,
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
* @deprecated use {@link #forcemerge(ForceMergeRequest, RequestOptions)} instead
*/
@Deprecated
public ForceMergeResponse forceMerge(ForceMergeRequest forceMergeRequest, RequestOptions options) throws IOException {
return forcemerge(forceMergeRequest, options);
}

/**
* Force merge one or more indices using the Force Merge API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-forcemerge.html">
* Force Merge API on elastic.co</a>
* @param forceMergeRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public ForceMergeResponse forcemerge(ForceMergeRequest forceMergeRequest, RequestOptions options) throws IOException {
Copy link
Member

@nik9000 nik9000 Jul 5, 2018

Choose a reason for hiding this comment

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

ew

return restHighLevelClient.performRequestAndParseEntity(forceMergeRequest, RequestConverters::forceMerge, options,
ForceMergeResponse::fromXContent, emptySet());
}
Expand All @@ -487,8 +502,22 @@ public ForceMergeResponse forceMerge(ForceMergeRequest forceMergeRequest, Reques
* @param forceMergeRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
* @deprecated use {@link #forcemergeAsync(ForceMergeRequest, RequestOptions, ActionListener)} instead
*/
@Deprecated
public void forceMergeAsync(ForceMergeRequest forceMergeRequest, RequestOptions options, ActionListener<ForceMergeResponse> listener) {
forcemergeAsync(forceMergeRequest, options, listener);
}

/**
* Asynchronously force merge one or more indices using the Force Merge API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-forcemerge.html">
* Force Merge API on elastic.co</a>
* @param forceMergeRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public void forcemergeAsync(ForceMergeRequest forceMergeRequest, RequestOptions options, ActionListener<ForceMergeResponse> listener) {
restHighLevelClient.performRequestAsyncAndParseEntity(forceMergeRequest, RequestConverters::forceMerge, options,
ForceMergeResponse::fromXContent, listener, emptySet());
}
Expand Down
8 changes: 4 additions & 4 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/IngestClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void deletePipelineAsync(DeletePipelineRequest request, RequestOptions op
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public SimulatePipelineResponse simulatePipeline(SimulatePipelineRequest request, RequestOptions options) throws IOException {
public SimulatePipelineResponse simulate(SimulatePipelineRequest request, RequestOptions options) throws IOException {
return restHighLevelClient.performRequestAndParseEntity( request, RequestConverters::simulatePipeline, options,
SimulatePipelineResponse::fromXContent, emptySet());
}
Expand All @@ -154,9 +154,9 @@ public SimulatePipelineResponse simulatePipeline(SimulatePipelineRequest request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public void simulatePipelineAsync(SimulatePipelineRequest request,
RequestOptions options,
ActionListener<SimulatePipelineResponse> listener) {
public void simulateAsync(SimulatePipelineRequest request,
RequestOptions options,
ActionListener<SimulatePipelineResponse> listener) {
restHighLevelClient.performRequestAsyncAndParseEntity( request, RequestConverters::simulatePipeline, options,
SimulatePipelineResponse::fromXContent, listener, emptySet());
}
Expand Down
102 changes: 95 additions & 7 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 @@ -384,8 +384,23 @@ public final void getAsync(GetRequest getRequest, RequestOptions options, Action
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
* @deprecated use {@link #mget(MultiGetRequest, RequestOptions)} instead
*/
@Deprecated
public final MultiGetResponse multiGet(MultiGetRequest multiGetRequest, RequestOptions options) throws IOException {
return mget(multiGetRequest, options);
}


/**
* Retrieves multiple documents by id using the Multi Get API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-multi-get.html">Multi Get API on elastic.co</a>
* @param multiGetRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public final MultiGetResponse mget(MultiGetRequest multiGetRequest, RequestOptions options) throws IOException {
return performRequestAndParseEntity(multiGetRequest, RequestConverters::multiGet, options, MultiGetResponse::fromXContent,
singleton(404));
}
Expand All @@ -396,8 +411,21 @@ public final MultiGetResponse multiGet(MultiGetRequest multiGetRequest, RequestO
* @param multiGetRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
* @deprecated use {@link #mgetAsync(MultiGetRequest, RequestOptions, ActionListener)} instead
*/
@Deprecated
public final void multiGetAsync(MultiGetRequest multiGetRequest, RequestOptions options, ActionListener<MultiGetResponse> listener) {
mgetAsync(multiGetRequest, options, listener);
}

/**
* Asynchronously retrieves multiple documents by id using the Multi Get API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-multi-get.html">Multi Get API on elastic.co</a>
* @param multiGetRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public final void mgetAsync(MultiGetRequest multiGetRequest, RequestOptions options, ActionListener<MultiGetResponse> listener) {
performRequestAsyncAndParseEntity(multiGetRequest, RequestConverters::multiGet, options, MultiGetResponse::fromXContent, listener,
singleton(404));
}
Expand Down Expand Up @@ -531,8 +559,23 @@ public final void searchAsync(SearchRequest searchRequest, RequestOptions option
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
* @deprecated use {@link #msearch(MultiSearchRequest, RequestOptions)} instead
*/
@Deprecated
public final MultiSearchResponse multiSearch(MultiSearchRequest multiSearchRequest, RequestOptions options) throws IOException {
return msearch(multiSearchRequest, options);
}

/**
* Executes a multi search using the msearch API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-multi-search.html">Multi search API on
* elastic.co</a>
* @param multiSearchRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public final MultiSearchResponse msearch(MultiSearchRequest multiSearchRequest, RequestOptions options) throws IOException {
return performRequestAndParseEntity(multiSearchRequest, RequestConverters::multiSearch, options, MultiSearchResponse::fromXContext,
emptySet());
}
Expand All @@ -544,9 +587,24 @@ public final MultiSearchResponse multiSearch(MultiSearchRequest multiSearchReque
* @param searchRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
* @deprecated use {@link #msearchAsync(MultiSearchRequest, RequestOptions, ActionListener)} instead
*/
@Deprecated
public final void multiSearchAsync(MultiSearchRequest searchRequest, RequestOptions options,
ActionListener<MultiSearchResponse> listener) {
ActionListener<MultiSearchResponse> listener) {
msearchAsync(searchRequest, options, listener);
}

/**
* Asynchronously executes a multi search using the msearch API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-multi-search.html">Multi search API on
* elastic.co</a>
* @param searchRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public final void msearchAsync(MultiSearchRequest searchRequest, RequestOptions options,
ActionListener<MultiSearchResponse> listener) {
performRequestAsyncAndParseEntity(searchRequest, RequestConverters::multiSearch, options, MultiSearchResponse::fromXContext,
listener, emptySet());
}
Expand All @@ -559,8 +617,23 @@ public final void multiSearchAsync(MultiSearchRequest searchRequest, RequestOpti
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
* @deprecated use {@link #scroll(SearchScrollRequest, RequestOptions)} instead
*/
@Deprecated
public final SearchResponse searchScroll(SearchScrollRequest searchScrollRequest, RequestOptions options) throws IOException {
return scroll(searchScrollRequest, options);
}

/**
* Executes a search using the Search Scroll API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-scroll.html">Search Scroll
* API on elastic.co</a>
* @param searchScrollRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public final SearchResponse scroll(SearchScrollRequest searchScrollRequest, RequestOptions options) throws IOException {
return performRequestAndParseEntity(searchScrollRequest, RequestConverters::searchScroll, options, SearchResponse::fromXContent,
emptySet());
}
Expand All @@ -572,9 +645,24 @@ public final SearchResponse searchScroll(SearchScrollRequest searchScrollRequest
* @param searchScrollRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
* @deprecated use {@link #scrollAsync(SearchScrollRequest, RequestOptions, ActionListener)} instead
*/
@Deprecated
public final void searchScrollAsync(SearchScrollRequest searchScrollRequest, RequestOptions options,
ActionListener<SearchResponse> listener) {
ActionListener<SearchResponse> listener) {
scrollAsync(searchScrollRequest, options, listener);
}

/**
* Asynchronously executes a search using the Search Scroll API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-scroll.html">Search Scroll
* API on elastic.co</a>
* @param searchScrollRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public final void scrollAsync(SearchScrollRequest searchScrollRequest, RequestOptions options,
ActionListener<SearchResponse> listener) {
performRequestAsyncAndParseEntity(searchScrollRequest, RequestConverters::searchScroll, options, SearchResponse::fromXContent,
listener, emptySet());
}
Expand Down Expand Up @@ -691,8 +779,8 @@ public final RankEvalResponse rankEval(RankEvalRequest rankEvalRequest, RequestO
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-search-template.html">Multi Search Template API
* on elastic.co</a>.
*/
public final MultiSearchTemplateResponse multiSearchTemplate(MultiSearchTemplateRequest multiSearchTemplateRequest,
RequestOptions options) throws IOException {
public final MultiSearchTemplateResponse msearchTemplate(MultiSearchTemplateRequest multiSearchTemplateRequest,
RequestOptions options) throws IOException {
return performRequestAndParseEntity(multiSearchTemplateRequest, RequestConverters::multiSearchTemplate,
options, MultiSearchTemplateResponse::fromXContext, emptySet());
}
Expand All @@ -703,9 +791,9 @@ public final MultiSearchTemplateResponse multiSearchTemplate(MultiSearchTemplate
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-search-template.html">Multi Search Template API
* on elastic.co</a>.
*/
public final void multiSearchTemplateAsync(MultiSearchTemplateRequest multiSearchTemplateRequest,
RequestOptions options,
ActionListener<MultiSearchTemplateResponse> listener) {
public final void msearchTemplateAsync(MultiSearchTemplateRequest multiSearchTemplateRequest,
RequestOptions options,
ActionListener<MultiSearchTemplateResponse> listener) {
performRequestAsyncAndParseEntity(multiSearchTemplateRequest, RequestConverters::multiSearchTemplate,
options, MultiSearchTemplateResponse::fromXContext, listener, emptySet());
}
Expand Down
Loading