From 5085a471e3bc57844aa145b2d00894bca30b2fef Mon Sep 17 00:00:00 2001 From: Pavlo Smahin Date: Thu, 13 Aug 2026 16:24:06 +0300 Subject: [PATCH] fix: improve DELETE_INCOMING handling for repeatable fields with duplicates (#437) - Ensure proper removal of duplicate scalar and object values in arrays. - Add unit tests to verify behavior with interspersed and consecutive duplicates. Closes: MODDICORE-507 --- .../mapper/writer/common/JsonBasedWriter.java | 12 ++- .../writer/JsonBasedWriterUnitTest.java | 76 +++++++++++++++++++ 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/folio/processing/mapping/mapper/writer/common/JsonBasedWriter.java b/src/main/java/org/folio/processing/mapping/mapper/writer/common/JsonBasedWriter.java index d86100cd..4d6c4582 100644 --- a/src/main/java/org/folio/processing/mapping/mapper/writer/common/JsonBasedWriter.java +++ b/src/main/java/org/folio/processing/mapping/mapper/writer/common/JsonBasedWriter.java @@ -384,7 +384,8 @@ private void deleteIncomingFieldByPath(ListValue listValue, JsonNode foundNode) if (foundNode != null && !foundNode.isEmpty()) { ArrayNode arrayNode = (ArrayNode) foundNode; int indexForDelete = 0; - for (int i = 0; i < arrayNode.size() + 1; i++) { + int initialSize = arrayNode.size(); + for (int i = 0; i < initialSize + 1; i++) { if (arrayNode.get(i - indexForDelete) != null && listValue.getValue() .contains(arrayNode.get(i - indexForDelete).textValue())) { arrayNode.remove(i - indexForDelete); @@ -403,11 +404,7 @@ private void removeIncomingArrayValues(JsonNode currentObject, String currentPat } private void removeMatchingObjectValues(JsonNode currentObject, ArrayNode arrayNode) { - for (int i = 0; i < arrayNode.size(); i++) { - if (arrayNode.get(i).equals(currentObject) || ifDeepEquals(currentObject, arrayNode.get(i))) { - arrayNode.remove(i); - } - } + arrayNode.removeIf(node -> ifDeepEquals(currentObject, node)); } /** @@ -424,7 +421,8 @@ private void removeScalarIncomingValues(JsonNode currentObject, String currentPa incomingArray.forEach(node -> incomingValues.add(node.textValue())); int indexForDelete = 0; - for (int i = 0; i < arrayNode.size() + 1; i++) { + int initialSize = arrayNode.size(); + for (int i = 0; i < initialSize + 1; i++) { JsonNode node = arrayNode.get(i - indexForDelete); if (node != null && incomingValues.contains(node.textValue())) { arrayNode.remove(i - indexForDelete); diff --git a/src/test/java/org/folio/processing/mapping/mapper/writer/JsonBasedWriterUnitTest.java b/src/test/java/org/folio/processing/mapping/mapper/writer/JsonBasedWriterUnitTest.java index 216d3b24..2448a603 100644 --- a/src/test/java/org/folio/processing/mapping/mapper/writer/JsonBasedWriterUnitTest.java +++ b/src/test/java/org/folio/processing/mapping/mapper/writer/JsonBasedWriterUnitTest.java @@ -169,6 +169,82 @@ void shouldWrite_RepeatableDeleteIncomingValuesInStringArray() throws IOExceptio assertEquals("{\"instance\":{\"administrativeNotes\":[\"Test2\",\"Test3\"]}}", resultInstance); } + @Test + void shouldDeleteAllMatchingEntries_whenDeleteIncomingWithInterspersedDuplicates() throws IOException { + // given + DataImportEventPayload eventContext = new DataImportEventPayload(); + HashMap context = new HashMap<>(); + context.put(EntityType.INSTANCE.value(), + """ + { + "instance": { + "administrativeNotes": ["Keep1", "Delete", "Keep2", "Delete", "Delete", "Keep3", "Delete"] + } + } + """); + eventContext.setContext(context); + // when + WRITER.initialize(eventContext); + WRITER.write("instance.administrativeNotes[]", ListValue.of(List.of("Delete"), DELETE_INCOMING)); + WRITER.getResult(eventContext); + // then + String resultInstance = eventContext.getContext().get(EntityType.INSTANCE.value()); + assertEquals("{\"instance\":{\"administrativeNotes\":[\"Keep1\",\"Keep2\",\"Keep3\"]}}", resultInstance); + } + + @Test + void shouldDeleteAllConsecutiveDuplicates_whenDeleteIncomingObjectsAreConsecutive() throws IOException { + // given + DataImportEventPayload eventContext = new DataImportEventPayload(); + HashMap context = new HashMap<>(); + context.put(EntityType.INSTANCE.value(), + """ + { + "instance": { + "contributor": [{"id":"1"},{"id":"2"},{"id":"2"},{"id":"3"}] + } + } + """); + eventContext.setContext(context); + // when + WRITER.initialize(eventContext); + List> objects = List.of(Map.of("instance.contributor[].id", StringValue.of("2"))); + RepeatableFieldValue field = + RepeatableFieldValue.of(objects, MappingRule.RepeatableFieldAction.DELETE_INCOMING, "contributor"); + WRITER.write("instance.contributor[]", field); + WRITER.getResult(eventContext); + // then + String resultInstance = eventContext.getContext().get(EntityType.INSTANCE.value()); + assertEquals("{\"instance\":{\"contributor\":[{\"id\":\"1\"},{\"id\":\"3\"}]}}", resultInstance); + } + + @Test + void shouldDeleteAllMatchingScalars_whenDeleteIncomingViaRepeatableFieldWithInterspersed() throws IOException { + // given + DataImportEventPayload eventContext = new DataImportEventPayload(); + HashMap context = new HashMap<>(); + context.put(EntityType.INSTANCE.value(), + """ + { + "instance": { + "administrativeNotes": ["Delete","Keep","Delete","Delete","Keep","Delete"] + } + } + """); + eventContext.setContext(context); + // when + WRITER.initialize(eventContext); + List> values = + List.of(Map.of("instance.administrativeNotes[]", ListValue.of(List.of("Delete")))); + RepeatableFieldValue field = + RepeatableFieldValue.of(values, MappingRule.RepeatableFieldAction.DELETE_INCOMING, "administrativeNotes"); + WRITER.write("instance.administrativeNotes[]", field); + WRITER.getResult(eventContext); + // then + String resultInstance = eventContext.getContext().get(EntityType.INSTANCE.value()); + assertEquals("{\"instance\":{\"administrativeNotes\":[\"Keep\",\"Keep\"]}}", resultInstance); + } + @Test void shouldWrite_RepeatableDeleteIncomingValuesIfThereAreSomeAdditionalFieldsExistsInEntity() throws IOException { // given