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

[Remove] Types from GET/MGET #2168

Merged
merged 7 commits into from
Feb 21, 2022
Merged

[Remove] Types from GET/MGET #2168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
053f00f
[Remove] Types from GET/MGET
nknize Feb 18, 2022
57eb3d8
fix test failures
nknize Feb 18, 2022
c540dd4
fix more failing tests w/ typos
nknize Feb 18, 2022
c00e0e9
Fix GetResultTests mutation
nknize Feb 18, 2022
1624b8d
more test failure fixes
nknize Feb 18, 2022
2173446
fix internalClusterTests
nknize Feb 18, 2022
8c02c32
revert codec test
nknize Feb 18, 2022
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
14 changes: 6 additions & 8 deletions client/rest-high-level/src/main/java/org/opensearch/client/RequestConverters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ static Request get(GetRequest getRequest) {
}

private static Request getStyleRequest(String method, GetRequest getRequest) {
Request request = new Request(method, endpoint(getRequest.index(), getRequest.type(), getRequest.id()));
Request request = new Request(method, endpoint(getRequest.index(), getRequest.id()));

Params parameters = new Params();
parameters.withPreference(getRequest.preference());
Expand Down Expand Up @@ -315,13 +315,7 @@ private static Request sourceRequest(GetSourceRequest getSourceRequest, String h
parameters.withRealtime(getSourceRequest.realtime());
parameters.withFetchSourceContext(getSourceRequest.fetchSourceContext());

String optionalType = getSourceRequest.type();
String endpoint;
if (optionalType == null) {
endpoint = endpoint(getSourceRequest.index(), "_source", getSourceRequest.id());
} else {
endpoint = endpoint(getSourceRequest.index(), optionalType, getSourceRequest.id(), "_source");
}
String endpoint = endpoint(getSourceRequest.index(), "_source", getSourceRequest.id());
Request request = new Request(httpMethodName, endpoint);
request.addParameters(parameters.asMap());
return request;
Expand Down Expand Up @@ -799,6 +793,10 @@ static HttpEntity createEntity(ToXContent toXContent, XContentType xContentType,
return new NByteArrayEntity(source.bytes, source.offset, source.length, createContentType(xContentType));
}

static String endpoint(String index, String id) {
return new EndpointBuilder().addPathPart(index, MapperService.SINGLE_MAPPING_NAME, id).build();
}

@Deprecated
static String endpoint(String index, String type, String id) {
return new EndpointBuilder().addPathPart(index, type, id).build();
Expand Down
1 change: 0 additions & 1 deletion client/rest-high-level/src/test/java/org/opensearch/client/BulkProcessorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,6 @@ private static void assertMultiGetResponse(MultiGetResponse multiGetResponse, in
int i = 1;
for (MultiGetItemResponse multiGetItemResponse : multiGetResponse) {
assertThat(multiGetItemResponse.getIndex(), equalTo("test"));
assertThat(multiGetItemResponse.getType(), equalTo("_doc"));
assertThat(multiGetItemResponse.getId(), equalTo(Integer.toString(i++)));
}
}
Expand Down
34 changes: 2 additions & 32 deletions client/rest-high-level/src/test/java/org/opensearch/client/CrudIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@
import org.opensearch.index.get.GetResult;
import org.opensearch.rest.RestStatus;
import org.opensearch.rest.action.document.RestBulkAction;
import org.opensearch.rest.action.document.RestMultiGetAction;
import org.opensearch.script.Script;
import org.opensearch.script.ScriptType;
import org.opensearch.search.fetch.subphase.FetchSourceContext;
Expand Down Expand Up @@ -337,7 +336,6 @@ public void testGet() throws IOException {
}
GetResponse getResponse = execute(getRequest, highLevelClient()::get, highLevelClient()::getAsync);
assertEquals("index", getResponse.getIndex());
assertEquals("_doc", getResponse.getType());
assertEquals("id", getResponse.getId());
assertTrue(getResponse.isExists());
assertFalse(getResponse.isSourceEmpty());
Expand All @@ -348,7 +346,6 @@ public void testGet() throws IOException {
GetRequest getRequest = new GetRequest("index", "does_not_exist");
GetResponse getResponse = execute(getRequest, highLevelClient()::get, highLevelClient()::getAsync);
assertEquals("index", getResponse.getIndex());
assertEquals("_doc", getResponse.getType());
assertEquals("does_not_exist", getResponse.getId());
assertFalse(getResponse.isExists());
assertEquals(-1, getResponse.getVersion());
Expand All @@ -360,7 +357,6 @@ public void testGet() throws IOException {
getRequest.fetchSourceContext(new FetchSourceContext(false, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY));
GetResponse getResponse = execute(getRequest, highLevelClient()::get, highLevelClient()::getAsync);
assertEquals("index", getResponse.getIndex());
assertEquals("_doc", getResponse.getType());
assertEquals("id", getResponse.getId());
assertTrue(getResponse.isExists());
assertTrue(getResponse.isSourceEmpty());
Expand All @@ -376,7 +372,6 @@ public void testGet() throws IOException {
}
GetResponse getResponse = execute(getRequest, highLevelClient()::get, highLevelClient()::getAsync);
assertEquals("index", getResponse.getIndex());
assertEquals("_doc", getResponse.getType());
assertEquals("id", getResponse.getId());
assertTrue(getResponse.isExists());
assertFalse(getResponse.isSourceEmpty());
Expand All @@ -398,7 +393,6 @@ public void testMultiGet() throws IOException {
assertTrue(response.getResponses()[0].isFailed());
assertNull(response.getResponses()[0].getResponse());
assertEquals("id1", response.getResponses()[0].getFailure().getId());
assertNull(response.getResponses()[0].getFailure().getType());
assertEquals("index", response.getResponses()[0].getFailure().getIndex());
assertEquals(
"OpenSearch exception [type=index_not_found_exception, reason=no such index [index]]",
Expand All @@ -408,7 +402,6 @@ public void testMultiGet() throws IOException {
assertTrue(response.getResponses()[1].isFailed());
assertNull(response.getResponses()[1].getResponse());
assertEquals("id2", response.getResponses()[1].getId());
assertNull(response.getResponses()[1].getType());
assertEquals("index", response.getResponses()[1].getIndex());
assertEquals(
"OpenSearch exception [type=index_not_found_exception, reason=no such index [index]]",
Expand All @@ -434,14 +427,12 @@ public void testMultiGet() throws IOException {
assertFalse(response.getResponses()[0].isFailed());
assertNull(response.getResponses()[0].getFailure());
assertEquals("id1", response.getResponses()[0].getId());
assertEquals("_doc", response.getResponses()[0].getType());
assertEquals("index", response.getResponses()[0].getIndex());
assertEquals(Collections.singletonMap("field", "value1"), response.getResponses()[0].getResponse().getSource());

assertFalse(response.getResponses()[1].isFailed());
assertNull(response.getResponses()[1].getFailure());
assertEquals("id2", response.getResponses()[1].getId());
assertEquals("_doc", response.getResponses()[1].getType());
assertEquals("index", response.getResponses()[1].getIndex());
assertEquals(Collections.singletonMap("field", "value2"), response.getResponses()[1].getResponse().getSource());
}
Expand All @@ -456,25 +447,7 @@ public void testMultiGetWithTypes() throws IOException {
highLevelClient().bulk(bulk, expectWarningsOnce(RestBulkAction.TYPES_DEPRECATION_MESSAGE));
MultiGetRequest multiGetRequest = new MultiGetRequest();
multiGetRequest.add("index", "id1");
multiGetRequest.add("index", "type", "id2");

MultiGetResponse response = execute(
multiGetRequest,
highLevelClient()::mget,
highLevelClient()::mgetAsync,
expectWarningsOnce(RestMultiGetAction.TYPES_DEPRECATION_MESSAGE)
);
assertEquals(2, response.getResponses().length);

GetResponse firstResponse = response.getResponses()[0].getResponse();
assertEquals("index", firstResponse.getIndex());
assertEquals("type", firstResponse.getType());
assertEquals("id1", firstResponse.getId());

GetResponse secondResponse = response.getResponses()[1].getResponse();
assertEquals("index", secondResponse.getIndex());
assertEquals("type", secondResponse.getType());
assertEquals("id2", secondResponse.getId());
multiGetRequest.add("index", "id2");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor, but the test name testMultiGetWithTypes() is not reflecting anymore what the test is doing, testMultiGetWithIds() ?

Copy link
Member

Choose a reason for hiding this comment

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

@nknize ^

}

public void testGetSource() throws IOException {
Expand Down Expand Up @@ -509,7 +482,7 @@ public void testGetSource() throws IOException {
);
assertEquals(RestStatus.NOT_FOUND, exception.status());
assertEquals(
"OpenSearch exception [type=resource_not_found_exception, " + "reason=Document not found [index]/[_doc]/[does_not_exist]]",
"OpenSearch exception [type=resource_not_found_exception, " + "reason=Document not found [index]/[does_not_exist]]",
exception.getMessage()
);
}
Expand Down Expand Up @@ -1077,7 +1050,6 @@ public void testUrlEncode() throws IOException {
GetResponse getResponse = highLevelClient().get(getRequest, RequestOptions.DEFAULT);
assertTrue(getResponse.isExists());
assertEquals(expectedIndex, getResponse.getIndex());
assertEquals("_doc", getResponse.getType());
assertEquals("id#1", getResponse.getId());
}

Expand All @@ -1095,7 +1067,6 @@ public void testUrlEncode() throws IOException {
GetResponse getResponse = highLevelClient().get(getRequest, RequestOptions.DEFAULT);
assertTrue(getResponse.isExists());
assertEquals("index", getResponse.getIndex());
assertEquals("_doc", getResponse.getType());
assertEquals(docId, getResponse.getId());
}

Expand All @@ -1119,7 +1090,6 @@ public void testParamsEncode() throws IOException {
GetResponse getResponse = highLevelClient().get(getRequest, RequestOptions.DEFAULT);
assertTrue(getResponse.isExists());
assertEquals("index", getResponse.getIndex());
assertEquals("_doc", getResponse.getType());
assertEquals("id", getResponse.getId());
assertEquals(routing, getResponse.getField("_routing").getValue());
}
Expand Down
30 changes: 2 additions & 28 deletions client/rest-high-level/src/test/java/org/opensearch/client/RequestConvertersTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,6 @@ public void testGet() {
getAndExistsTest(RequestConverters::get, HttpGet.METHOD_NAME);
}

public void testGetWithType() {
getAndExistsWithTypeTest(RequestConverters::get, HttpGet.METHOD_NAME);
}

public void testSourceExists() throws IOException {
doTestSourceExists((index, id) -> new GetSourceRequest(index, id));
}
Expand Down Expand Up @@ -221,13 +217,7 @@ private static void doTestSourceExists(BiFunction<String, String, GetSourceReque
}
Request request = RequestConverters.sourceExists(getRequest);
assertEquals(HttpHead.METHOD_NAME, request.getMethod());
String type = getRequest.type();
if (type == null) {
assertEquals("/" + index + "/_source/" + id, request.getEndpoint());
} else {
assertEquals("/" + index + "/" + type + "/" + id + "/_source", request.getEndpoint());
}

assertEquals("/" + index + "/_source/" + id, request.getEndpoint());
assertEquals(expectedParams, request.getParameters());
assertNull(request.getEntity());
}
Expand Down Expand Up @@ -321,7 +311,7 @@ public void testMultiGet() throws IOException {

public void testMultiGetWithType() throws IOException {
MultiGetRequest multiGetRequest = new MultiGetRequest();
MultiGetRequest.Item item = new MultiGetRequest.Item(randomAlphaOfLength(4), randomAlphaOfLength(4), randomAlphaOfLength(4));
MultiGetRequest.Item item = new MultiGetRequest.Item(randomAlphaOfLength(4), randomAlphaOfLength(4));
multiGetRequest.add(item);

Request request = RequestConverters.multiGet(multiGetRequest);
Expand Down Expand Up @@ -374,10 +364,6 @@ public void testExists() {
getAndExistsTest(RequestConverters::exists, HttpHead.METHOD_NAME);
}

public void testExistsWithType() {
getAndExistsWithTypeTest(RequestConverters::exists, HttpHead.METHOD_NAME);
}

private static void getAndExistsTest(Function<GetRequest, Request> requestConverter, String method) {
String index = randomAlphaOfLengthBetween(3, 10);
String id = randomAlphaOfLengthBetween(3, 10);
Expand Down Expand Up @@ -435,18 +421,6 @@ private static void getAndExistsTest(Function<GetRequest, Request> requestConver
assertEquals(method, request.getMethod());
}

private static void getAndExistsWithTypeTest(Function<GetRequest, Request> requestConverter, String method) {
String index = randomAlphaOfLengthBetween(3, 10);
String type = randomAlphaOfLengthBetween(3, 10);
String id = randomAlphaOfLengthBetween(3, 10);
GetRequest getRequest = new GetRequest(index, type, id);

Request request = requestConverter.apply(getRequest);
assertEquals("/" + index + "/" + type + "/" + id, request.getEndpoint());
assertNull(request.getEntity());
assertEquals(method, request.getMethod());
}

public void testReindex() throws IOException {
ReindexRequest reindexRequest = new ReindexRequest();
reindexRequest.setSourceIndices("source_idx");
Expand Down
1 change: 0 additions & 1 deletion ...est-high-level/src/test/java/org/opensearch/client/documentation/CRUDDocumentationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2050,7 +2050,6 @@ private MultiGetItemResponse unwrapAndAssertExample(MultiGetResponse response) {
assertThat(response.getResponses(), arrayWithSize(1));
MultiGetItemResponse item = response.getResponses()[0];
assertEquals("index", item.getIndex());
assertEquals("_doc", item.getType());
assertEquals("example_id", item.getId());
return item;
}
Expand Down
10 changes: 5 additions & 5 deletions ...est-common/src/internalClusterTest/java/org/opensearch/ingest/common/IngestRestartIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public Settings onNodeStopped(String nodeName) {
)
);

Map<String, Object> source = client().prepareGet("index", "doc", "1").get().getSource();
Map<String, Object> source = client().prepareGet("index", "1").get().getSource();
assertThat(source.get("x"), equalTo(0));
assertThat(source.get("y"), equalTo(0));
}
Expand Down Expand Up @@ -242,7 +242,7 @@ public void testPipelineWithScriptProcessorThatHasStoredScript() throws Exceptio
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
.get();

Map<String, Object> source = client().prepareGet("index", "doc", "1").get().getSource();
Map<String, Object> source = client().prepareGet("index", "1").get().getSource();
assertThat(source.get("x"), equalTo(0));
assertThat(source.get("y"), equalTo(0));
assertThat(source.get("z"), equalTo(0));
Expand All @@ -260,7 +260,7 @@ public void testPipelineWithScriptProcessorThatHasStoredScript() throws Exceptio
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
.get();

source = client().prepareGet("index", "doc", "2").get().getSource();
source = client().prepareGet("index", "2").get().getSource();
assertThat(source.get("x"), equalTo(0));
assertThat(source.get("y"), equalTo(0));
assertThat(source.get("z"), equalTo(0));
Expand All @@ -281,7 +281,7 @@ public void testWithDedicatedIngestNode() throws Exception {
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
.get();

Map<String, Object> source = client().prepareGet("index", "doc", "1").get().getSource();
Map<String, Object> source = client().prepareGet("index", "1").get().getSource();
assertThat(source.get("x"), equalTo(0));
assertThat(source.get("y"), equalTo(0));

Expand All @@ -294,7 +294,7 @@ public void testWithDedicatedIngestNode() throws Exception {
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
.get();

source = client(ingestNode).prepareGet("index", "doc", "2").get().getSource();
source = client(ingestNode).prepareGet("index", "2").get().getSource();
assertThat(source.get("x"), equalTo(0));
assertThat(source.get("y"), equalTo(0));
}
Expand Down
Loading