From 242e85a797da2c8f47ef8b359c36e53e1d93ce7b Mon Sep 17 00:00:00 2001 From: Imri Paran Date: Thu, 6 Mar 2025 15:00:01 +0100 Subject: [PATCH] fix(change-summary): minor issues (#20095) - deep copy previous change summary so that it doesnt get modified - handle multiple fields during consolidation --- .../service/jdbi3/ChangeSummarizer.java | 5 ++++- .../service/jdbi3/EntityRepository.java | 1 + .../service/jdbi3/ChangeSummarizerTest.java | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChangeSummarizer.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChangeSummarizer.java index 1044a65eeb9..f4e720a1db8 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChangeSummarizer.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChangeSummarizer.java @@ -56,7 +56,10 @@ public class ChangeSummarizer { new ChangeSummary() .withChangeSource(changeSource) .withChangedAt(changedAt) - .withChangedBy(changedBy))); + .withChangedBy(changedBy), + // If its a consolidation, we might have multiple changes for the same field. + // Since we are only interested in the field name, we can just take whichever. + (existing, replacement) -> existing)); } private boolean isFieldTracked(String fieldName) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java index f02a50e695c..f6171b08f02 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java @@ -2993,6 +2993,7 @@ public abstract class EntityRepository { ChangeSummaryMap current = Optional.ofNullable(original.getChangeDescription()) .map(ChangeDescription::getChangeSummary) + .map(changeSummaryMap -> JsonUtils.deepCopy(changeSummaryMap, ChangeSummaryMap.class)) .orElse(new ChangeSummaryMap()); Map addedSummaries = diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/ChangeSummarizerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/ChangeSummarizerTest.java index 94b20168317..3ec534bd448 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/ChangeSummarizerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/ChangeSummarizerTest.java @@ -42,6 +42,26 @@ public class ChangeSummarizerTest { assert result.size() == 0; } + @Test + public void test_duplicateEntriesConsolidation() { + String fieldName = "description"; + ChangeSource changeSource = ChangeSource.MANUAL; + long updatedAt = System.currentTimeMillis(); + String updatedBy = "testUser"; + List changes = + List.of(new FieldChange().withName(fieldName), new FieldChange().withName(fieldName)); + + Map result = + changeSummarizer.summarizeChanges(Map.of(), changes, changeSource, updatedBy, updatedAt); + assert result.size() == 1; + Assertions.assertTrue(result.containsKey(fieldName)); + + result = + changeSummarizer.summarizeChanges( + result, changes, ChangeSource.AUTOMATED, "older-change", updatedAt - 100); + assert result.size() == 0; + } + @Test public void test_columnDescription() { String fieldName = "columns.column1.description";