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 DocWrite Request and Response #2239

Merged
merged 4 commits into from
Feb 24, 2022
Merged

[Remove] Types from DocWrite Request and Response #2239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b1a658
[Remove] Types from DocWrite Request and Response
nknize Feb 24, 2022
c60f3b9
fix additional missed tests
nknize Feb 24, 2022
88ccab9
more test fixes
nknize Feb 24, 2022
1a1c3e2
PR Feedback
nknize Feb 24, 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
2 changes: 1 addition & 1 deletion ...p-api-plugin/src/main/java/org/opensearch/plugin/noop/action/bulk/RestNoopBulkAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ private static class BulkRestBuilderListener extends RestBuilderListener<BulkReq
private final BulkItemResponse ITEM_RESPONSE = new BulkItemResponse(
1,
DocWriteRequest.OpType.UPDATE,
new UpdateResponse(new ShardId("mock", "", 1), "mock_type", "1", 0L, 1L, 1L, DocWriteResponse.Result.CREATED)
new UpdateResponse(new ShardId("mock", "", 1), "1", 0L, 1L, 1L, DocWriteResponse.Result.CREATED)
);

private final RestRequest request;
Expand Down
2 changes: 1 addition & 1 deletion ...-plugin/src/main/java/org/opensearch/plugin/noop/action/bulk/TransportNoopBulkAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class TransportNoopBulkAction extends HandledTransportAction<BulkRequest,
private static final BulkItemResponse ITEM_RESPONSE = new BulkItemResponse(
1,
DocWriteRequest.OpType.UPDATE,
new UpdateResponse(new ShardId("mock", "", 1), "mock_type", "1", 0L, 1L, 1L, DocWriteResponse.Result.CREATED)
new UpdateResponse(new ShardId("mock", "", 1), "1", 0L, 1L, 1L, DocWriteResponse.Result.CREATED)
);

@Inject
Expand Down
17 changes: 4 additions & 13 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 @@ -117,7 +117,7 @@ private RequestConverters() {
}

static Request delete(DeleteRequest deleteRequest) {
String endpoint = endpoint(deleteRequest.index(), deleteRequest.type(), deleteRequest.id());
String endpoint = endpoint(deleteRequest.index(), deleteRequest.id());
Request request = new Request(HttpDelete.METHOD_NAME, endpoint);

Params parameters = new Params();
Expand Down Expand Up @@ -185,11 +185,6 @@ static Request bulk(BulkRequest bulkRequest) throws IOException {
if (Strings.hasLength(action.index())) {
metadata.field("_index", action.index());
}
if (Strings.hasLength(action.type())) {
if (MapperService.SINGLE_MAPPING_NAME.equals(action.type()) == false) {
metadata.field("_type", action.type());
}
}
if (Strings.hasLength(action.id())) {
metadata.field("_id", action.id());
}
Expand Down Expand Up @@ -338,11 +333,9 @@ static Request index(IndexRequest indexRequest) {

String endpoint;
if (indexRequest.opType() == DocWriteRequest.OpType.CREATE) {
endpoint = indexRequest.type().equals(MapperService.SINGLE_MAPPING_NAME)
? endpoint(indexRequest.index(), "_create", indexRequest.id())
: endpoint(indexRequest.index(), indexRequest.type(), indexRequest.id(), "_create");
endpoint = endpoint(indexRequest.index(), "_create", indexRequest.id());
} else {
endpoint = endpoint(indexRequest.index(), indexRequest.type(), indexRequest.id());
endpoint = endpoint(indexRequest.index(), indexRequest.id());
}

Request request = new Request(method, endpoint);
Expand Down Expand Up @@ -371,9 +364,7 @@ static Request ping() {
}

static Request update(UpdateRequest updateRequest) throws IOException {
String endpoint = updateRequest.type().equals(MapperService.SINGLE_MAPPING_NAME)
? endpoint(updateRequest.index(), "_update", updateRequest.id())
: endpoint(updateRequest.index(), updateRequest.type(), updateRequest.id(), "_update");
String endpoint = endpoint(updateRequest.index(), "_update", updateRequest.id());
Request request = new Request(HttpPost.METHOD_NAME, endpoint);

Params parameters = new Params();
Expand Down
31 changes: 8 additions & 23 deletions 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 @@ -359,8 +359,6 @@ public void testGlobalParametersAndBulkProcessor() throws Exception {
{
final CountDownLatch latch = new CountDownLatch(1);
BulkProcessorTestListener listener = new BulkProcessorTestListener(latch);
// Check that untyped document additions inherit the global type
String localType = null;
try (
BulkProcessor processor = initBulkProcessorBuilder(listener)
// let's make sure that the bulk action limit trips, one single execution will index all the documents
Expand All @@ -374,7 +372,7 @@ public void testGlobalParametersAndBulkProcessor() throws Exception {
.build()
) {

indexDocs(processor, numDocs, null, localType, "test", "pipeline_id");
indexDocs(processor, numDocs, null, "test", "pipeline_id");
latch.await();

assertThat(listener.beforeCounts.get(), equalTo(1));
Expand All @@ -395,44 +393,31 @@ private Matcher<SearchHit>[] expectedIds(int numDocs) {
return IntStream.rangeClosed(1, numDocs).boxed().map(n -> hasId(n.toString())).<Matcher<SearchHit>>toArray(Matcher[]::new);
}

private MultiGetRequest indexDocs(
BulkProcessor processor,
int numDocs,
String localIndex,
String localType,
String globalIndex,
String globalPipeline
) throws Exception {
private MultiGetRequest indexDocs(BulkProcessor processor, int numDocs, String localIndex, String globalIndex, String globalPipeline)
throws Exception {
MultiGetRequest multiGetRequest = new MultiGetRequest();
for (int i = 1; i <= numDocs; i++) {
if (randomBoolean()) {
processor.add(
new IndexRequest(localIndex, localType, Integer.toString(i)).source(
XContentType.JSON,
"field",
randomRealisticUnicodeOfLengthBetween(1, 30)
)
new IndexRequest(localIndex).id(Integer.toString(i))
.source(XContentType.JSON, "field", randomRealisticUnicodeOfLengthBetween(1, 30))
);
} else {
BytesArray data = bytesBulkRequest(localIndex, localType, i);
BytesArray data = bytesBulkRequest(localIndex, i);
processor.add(data, globalIndex, globalPipeline, XContentType.JSON);
}
multiGetRequest.add(localIndex, Integer.toString(i));
}
return multiGetRequest;
}

private static BytesArray bytesBulkRequest(String localIndex, String localType, int id) throws IOException {
private static BytesArray bytesBulkRequest(String localIndex, int id) throws IOException {
XContentBuilder action = jsonBuilder().startObject().startObject("index");

if (localIndex != null) {
action.field("_index", localIndex);
}

if (localType != null) {
action.field("_type", localType);
}

action.field("_id", Integer.toString(id));
action.endObject().endObject();

Expand All @@ -443,7 +428,7 @@ private static BytesArray bytesBulkRequest(String localIndex, String localType,
}

private MultiGetRequest indexDocs(BulkProcessor processor, int numDocs) throws Exception {
return indexDocs(processor, numDocs, "test", null, null, null);
return indexDocs(processor, numDocs, "test", null, null);
}

private static void assertResponseItems(List<BulkItemResponse> bulkItemResponses, int numDocs) {
Expand Down
21 changes: 3 additions & 18 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 @@ -108,7 +108,6 @@ public void testDelete() throws IOException {
}
DeleteResponse deleteResponse = execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync);
assertEquals("index", deleteResponse.getIndex());
assertEquals("_doc", deleteResponse.getType());
assertEquals(docId, deleteResponse.getId());
assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult());
}
Expand All @@ -118,7 +117,6 @@ public void testDelete() throws IOException {
DeleteRequest deleteRequest = new DeleteRequest("index", docId);
DeleteResponse deleteResponse = execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync);
assertEquals("index", deleteResponse.getIndex());
assertEquals("_doc", deleteResponse.getType());
assertEquals(docId, deleteResponse.getId());
assertEquals(DocWriteResponse.Result.NOT_FOUND, deleteResponse.getResult());
}
Expand Down Expand Up @@ -157,7 +155,6 @@ public void testDelete() throws IOException {
DeleteRequest deleteRequest = new DeleteRequest("index", docId).versionType(VersionType.EXTERNAL).version(13);
DeleteResponse deleteResponse = execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync);
assertEquals("index", deleteResponse.getIndex());
assertEquals("_doc", deleteResponse.getType());
assertEquals(docId, deleteResponse.getId());
assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult());
}
Expand Down Expand Up @@ -194,7 +191,6 @@ public void testDelete() throws IOException {
DeleteRequest deleteRequest = new DeleteRequest("index", docId).routing("foo");
DeleteResponse deleteResponse = execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync);
assertEquals("index", deleteResponse.getIndex());
assertEquals("_doc", deleteResponse.getType());
assertEquals(docId, deleteResponse.getId());
assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult());
}
Expand Down Expand Up @@ -440,8 +436,8 @@ public void testMultiGet() throws IOException {
public void testMultiGetWithIds() throws IOException {
BulkRequest bulk = new BulkRequest();
bulk.setRefreshPolicy(RefreshPolicy.IMMEDIATE);
bulk.add(new IndexRequest("index", "id1").source("{\"field\":\"value1\"}", XContentType.JSON));
bulk.add(new IndexRequest("index", "id2").source("{\"field\":\"value2\"}", XContentType.JSON));
bulk.add(new IndexRequest("index").id("id1").source("{\"field\":\"value1\"}", XContentType.JSON));
bulk.add(new IndexRequest("index").id("id2").source("{\"field\":\"value2\"}", XContentType.JSON));

MultiGetRequest multiGetRequest = new MultiGetRequest();
multiGetRequest.add("index", "id1");
Expand Down Expand Up @@ -534,7 +530,6 @@ public void testIndex() throws IOException {
assertEquals(RestStatus.CREATED, indexResponse.status());
assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult());
assertEquals("index", indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertTrue(Strings.hasLength(indexResponse.getId()));
assertEquals(1L, indexResponse.getVersion());
assertNotNull(indexResponse.getShardId());
Expand All @@ -554,7 +549,6 @@ public void testIndex() throws IOException {
IndexResponse indexResponse = execute(indexRequest, highLevelClient()::index, highLevelClient()::indexAsync);
assertEquals(RestStatus.CREATED, indexResponse.status());
assertEquals("index", indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertEquals("id", indexResponse.getId());
assertEquals(1L, indexResponse.getVersion());

Expand All @@ -564,7 +558,6 @@ public void testIndex() throws IOException {
indexResponse = execute(indexRequest, highLevelClient()::index, highLevelClient()::indexAsync);
assertEquals(RestStatus.OK, indexResponse.status());
assertEquals("index", indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertEquals("id", indexResponse.getId());
assertEquals(2L, indexResponse.getVersion());

Expand Down Expand Up @@ -622,7 +615,6 @@ public void testIndex() throws IOException {
IndexResponse indexResponse = execute(indexRequest, highLevelClient()::index, highLevelClient()::indexAsync);
assertEquals(RestStatus.CREATED, indexResponse.status());
assertEquals("index", indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertEquals("external_version_type", indexResponse.getId());
assertEquals(12L, indexResponse.getVersion());
}
Expand All @@ -634,7 +626,6 @@ public void testIndex() throws IOException {
IndexResponse indexResponse = execute(indexRequest, highLevelClient()::index, highLevelClient()::indexAsync);
assertEquals(RestStatus.CREATED, indexResponse.status());
assertEquals("index", indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertEquals("with_create_op_type", indexResponse.getId());

OpenSearchStatusException exception = expectThrows(
Expand Down Expand Up @@ -662,7 +653,7 @@ public void testUpdate() throws IOException {
);
assertEquals(RestStatus.NOT_FOUND, exception.status());
assertEquals(
"OpenSearch exception [type=document_missing_exception, reason=[_doc][does_not_exist]: document missing]",
"OpenSearch exception [type=document_missing_exception, reason=[does_not_exist]: document missing]",
exception.getMessage()
);
}
Expand Down Expand Up @@ -787,7 +778,6 @@ public void testUpdate() throws IOException {
UpdateResponse updateResponse = execute(updateRequest, highLevelClient()::update, highLevelClient()::updateAsync);
assertEquals(RestStatus.CREATED, updateResponse.status());
assertEquals("index", updateResponse.getIndex());
assertEquals("_doc", updateResponse.getType());
assertEquals("with_upsert", updateResponse.getId());
GetResult getResult = updateResponse.getGetResult();
assertEquals(1L, updateResponse.getVersion());
Expand All @@ -802,7 +792,6 @@ public void testUpdate() throws IOException {
UpdateResponse updateResponse = execute(updateRequest, highLevelClient()::update, highLevelClient()::updateAsync);
assertEquals(RestStatus.CREATED, updateResponse.status());
assertEquals("index", updateResponse.getIndex());
assertEquals("_doc", updateResponse.getType());
assertEquals("with_doc_as_upsert", updateResponse.getId());
GetResult getResult = updateResponse.getGetResult();
assertEquals(1L, updateResponse.getVersion());
Expand All @@ -818,7 +807,6 @@ public void testUpdate() throws IOException {
UpdateResponse updateResponse = execute(updateRequest, highLevelClient()::update, highLevelClient()::updateAsync);
assertEquals(RestStatus.CREATED, updateResponse.status());
assertEquals("index", updateResponse.getIndex());
assertEquals("_doc", updateResponse.getType());
assertEquals("with_scripted_upsert", updateResponse.getId());

GetResult getResult = updateResponse.getGetResult();
Expand Down Expand Up @@ -1039,7 +1027,6 @@ public void testUrlEncode() throws IOException {
indexRequest.source("field", "value");
IndexResponse indexResponse = highLevelClient().index(indexRequest, RequestOptions.DEFAULT);
assertEquals(expectedIndex, indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertEquals("id#1", indexResponse.getId());
}
{
Expand All @@ -1056,7 +1043,6 @@ public void testUrlEncode() throws IOException {
indexRequest.source("field", "value");
IndexResponse indexResponse = highLevelClient().index(indexRequest, RequestOptions.DEFAULT);
assertEquals("index", indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertEquals(docId, indexResponse.getId());
}
{
Expand All @@ -1079,7 +1065,6 @@ public void testParamsEncode() throws IOException {
indexRequest.routing(routing);
IndexResponse indexResponse = highLevelClient().index(indexRequest, RequestOptions.DEFAULT);
assertEquals("index", indexResponse.getIndex());
assertEquals("_doc", indexResponse.getType());
assertEquals("id", indexResponse.getId());
}
{
Expand Down
Loading