From 2307573731baba8ef98387d235c49d27762bf52d Mon Sep 17 00:00:00 2001 From: Mohammad Hosseini Date: Mon, 30 May 2022 20:57:36 +0200 Subject: [PATCH] SE-5725: avoid duplication on upsert --- .../action/update/UpdateHelper.java | 19 ++++++++++++++++--- .../action/update/UpdateRequestTests.java | 6 +++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/update/UpdateHelper.java b/server/src/main/java/org/opensearch/action/update/UpdateHelper.java index 685b21b892fb3..bffb9e815c0e6 100644 --- a/server/src/main/java/org/opensearch/action/update/UpdateHelper.java +++ b/server/src/main/java/org/opensearch/action/update/UpdateHelper.java @@ -63,6 +63,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.UUID; import java.util.function.LongSupplier; /** @@ -101,7 +102,7 @@ protected Result prepare(ShardId shardId, UpdateRequest request, final GetResult throw new DocumentSourceMissingException(shardId, request.id()); } else if (request.script() == null && request.doc() != null) { // The request has no script, it is a new doc that should be merged with the old document - return prepareUpdateIndexRequest(shardId, request, getResult, request.detectNoop()); + return prepareUpdateIndexRequest(shardId, request, getResult, request.detectNoop(), nowInMillis); } else { // The request has a script (or empty script), execute the script and prepare a new index request return prepareUpdateScriptRequest(shardId, request, getResult, nowInMillis); @@ -206,14 +207,26 @@ static String calculateRouting(GetResult getResult, @Nullable IndexRequest updat * Prepare the request for merging the existing document with a new one, can optionally detect a noop change. Returns a {@code Result} * containing a new {@code IndexRequest} to be executed on the primary and replicas. */ - Result prepareUpdateIndexRequest(ShardId shardId, UpdateRequest request, GetResult getResult, boolean detectNoop) { + Result prepareUpdateIndexRequest(ShardId shardId, UpdateRequest request, GetResult getResult, boolean detectNoop, LongSupplier nowInMillis) { final IndexRequest currentRequest = request.doc(); final String routing = calculateRouting(getResult, currentRequest); final Tuple> sourceAndContent = XContentHelper.convertToMap(getResult.internalSourceRef(), true); final XContentType updateSourceContentType = sourceAndContent.v1(); final Map updatedSourceAsMap = sourceAndContent.v2(); + var requestSourceMap = currentRequest.sourceAsMap(); + + // In order to avoid updating the doc with the same id and fix the data migration the following code has been added. + var existingType = (String) updatedSourceAsMap.get("type"); + var newType = requestSourceMap.get("type"); + if (existingType != null && !existingType.equalsIgnoreCase((String) newType)) { + var newId = UUID.randomUUID().toString(); + logger.warn("we have the same doc [{}] with different type [{}], new type [{}], new id [{}]", request.id(), existingType, newType, newId); + request.id(newId); + currentRequest.id(newId); + return prepareUpsert(shardId, request, getResult, nowInMillis); + } - final boolean noop = !XContentHelper.update(updatedSourceAsMap, currentRequest.sourceAsMap(), detectNoop); + final boolean noop = !XContentHelper.update(updatedSourceAsMap, requestSourceMap, detectNoop); // We can only actually turn the update into a noop if detectNoop is true to preserve backwards compatibility and to handle cases // where users repopulating multi-fields or adding synonyms, etc. diff --git a/server/src/test/java/org/opensearch/action/update/UpdateRequestTests.java b/server/src/test/java/org/opensearch/action/update/UpdateRequestTests.java index 380b0628147de..2503ca00eb739 100644 --- a/server/src/test/java/org/opensearch/action/update/UpdateRequestTests.java +++ b/server/src/test/java/org/opensearch/action/update/UpdateRequestTests.java @@ -582,13 +582,13 @@ public void testNoopDetection() throws Exception { createParser(JsonXContent.jsonXContent, new BytesArray("{\"doc\": {\"body\": \"foo\"}}")) ); - UpdateHelper.Result result = updateHelper.prepareUpdateIndexRequest(shardId, request, getResult, true); + UpdateHelper.Result result = updateHelper.prepareUpdateIndexRequest(shardId, request, getResult, true, System::currentTimeMillis); assertThat(result.action(), instanceOf(UpdateResponse.class)); assertThat(result.getResponseResult(), equalTo(DocWriteResponse.Result.NOOP)); // Try again, with detectNoop turned off - result = updateHelper.prepareUpdateIndexRequest(shardId, request, getResult, false); + result = updateHelper.prepareUpdateIndexRequest(shardId, request, getResult, false, System::currentTimeMillis); assertThat(result.action(), instanceOf(IndexRequest.class)); assertThat(result.getResponseResult(), equalTo(DocWriteResponse.Result.UPDATED)); assertThat(result.updatedSourceAsMap().get("body").toString(), equalTo("foo")); @@ -597,7 +597,7 @@ public void testNoopDetection() throws Exception { request = new UpdateRequest("test", "1").fromXContent( createParser(JsonXContent.jsonXContent, new BytesArray("{\"doc\": {\"body\": \"bar\"}}")) ); - result = updateHelper.prepareUpdateIndexRequest(shardId, request, getResult, true); + result = updateHelper.prepareUpdateIndexRequest(shardId, request, getResult, true, System::currentTimeMillis); assertThat(result.action(), instanceOf(IndexRequest.class)); assertThat(result.getResponseResult(), equalTo(DocWriteResponse.Result.UPDATED));