From 46a35d075cd3bd7836e2df6e3da3dacb94c92264 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Mon, 14 Jul 2025 15:01:49 -0700 Subject: [PATCH] Fix multiple relation batch update (#22352) * Fix multiple relationship bulk add * Remove unused code --- .../service/jdbi3/EntityRepository.java | 59 ++++++++++++------- .../resources/metrics/MetricResourceTest.java | 38 ++++++++++++ 2 files changed, 75 insertions(+), 22 deletions(-) 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 0c944f818d4..58509b638fe 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 @@ -4222,30 +4222,45 @@ public abstract class EntityRepository { // Batch add new relationships if (!added.isEmpty()) { - // Create forward relationships - List addedIds = - added.stream().map(EntityReference::getId).collect(Collectors.toList()); - daoCollection - .relationshipDAO() - .bulkInsertToRelationship( - fromId, addedIds, fromEntityType, toEntityType, relationshipType.ordinal()); - if (bidirectional) { - // Create reverse relationships using bulkInsertTo for true batch operation - List reverseRelationships = - added.stream() - .map( - ref -> - CollectionDAO.EntityRelationshipObject.builder() - .fromId(ref.getId().toString()) - .toId(fromId.toString()) - .fromEntity(ref.getType()) - .toEntity(fromEntityType) - .relation(relationshipType.ordinal()) - .build()) - .collect(Collectors.toList()); + // For bidirectional relationships, apply the optimization where smaller UUID is always + // fromId + List optimizedRelationships = new ArrayList<>(); + for (EntityReference ref : added) { + UUID id1 = fromId; + UUID id2 = ref.getId(); + String entity1 = fromEntityType; + String entity2 = ref.getType(); - daoCollection.relationshipDAO().bulkInsertTo(reverseRelationships); + // Ensure smaller UUID is always fromId for bidirectional relationships + if (id1.compareTo(id2) > 0) { + // Swap the IDs and entities + UUID tempId = id1; + id1 = id2; + id2 = tempId; + String tempEntity = entity1; + entity1 = entity2; + entity2 = tempEntity; + } + + optimizedRelationships.add( + CollectionDAO.EntityRelationshipObject.builder() + .fromId(id1.toString()) + .toId(id2.toString()) + .fromEntity(entity1) + .toEntity(entity2) + .relation(relationshipType.ordinal()) + .build()); + } + daoCollection.relationshipDAO().bulkInsertTo(optimizedRelationships); + } else { + // For non-bidirectional relationships, just create forward relationships + List addedIds = + added.stream().map(EntityReference::getId).collect(Collectors.toList()); + daoCollection + .relationshipDAO() + .bulkInsertToRelationship( + fromId, addedIds, fromEntityType, toEntityType, relationshipType.ordinal()); } } if (!nullOrEmpty(updatedToRefs)) { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/metrics/MetricResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/metrics/MetricResourceTest.java index 655c69c2199..040c6203aed 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/metrics/MetricResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/metrics/MetricResourceTest.java @@ -57,6 +57,43 @@ public class MetricResourceTest extends EntityResourceTest Metric2 = createEntity(createMetric, ADMIN_AUTH_HEADERS); } + @Test + void test_duplicateRelatedMetricsIssue() throws IOException { + CreateMetric createMetric1 = createRequest("test_metric_duplicate_1", "", "", null); + Metric metric1 = createEntity(createMetric1, ADMIN_AUTH_HEADERS); + + CreateMetric createMetric2 = createRequest("test_metric_duplicate_2", "", "", null); + Metric metric2 = createEntity(createMetric2, ADMIN_AUTH_HEADERS); + + Metric originalMetric2 = getMetric(metric2.getId(), "*", ADMIN_AUTH_HEADERS); + String origJson = JsonUtils.pojoToJson(originalMetric2); + + originalMetric2.setRelatedMetrics(List.of(metric1.getEntityReference())); + patchEntity(originalMetric2.getId(), origJson, originalMetric2, ADMIN_AUTH_HEADERS); + + Metric updatedMetric2 = getMetric(metric2.getId(), "relatedMetrics", ADMIN_AUTH_HEADERS); + + Assertions.assertNotNull(updatedMetric2.getRelatedMetrics()); + Assertions.assertEquals( + 1, + updatedMetric2.getRelatedMetrics().size(), + "Expected only 1 related metric, but found " + + updatedMetric2.getRelatedMetrics().size() + + ". Related metrics: " + + updatedMetric2.getRelatedMetrics()); + Assertions.assertEquals(metric1.getId(), updatedMetric2.getRelatedMetrics().getFirst().getId()); + + // Also verify that metric1 now has metric2 as a related metric (bidirectional) + Metric updatedMetric1 = getMetric(metric1.getId(), "relatedMetrics", ADMIN_AUTH_HEADERS); + Assertions.assertNotNull(updatedMetric1.getRelatedMetrics()); + Assertions.assertEquals( + 1, + updatedMetric1.getRelatedMetrics().size(), + "Expected only 1 related metric for the reverse relationship, but found " + + updatedMetric1.getRelatedMetrics().size()); + Assertions.assertEquals(metric2.getId(), updatedMetric1.getRelatedMetrics().getFirst().getId()); + } + @Test void patch_MetricEntity() throws IOException { // Create a new Metric with different fields @@ -177,6 +214,7 @@ public class MetricResourceTest extends EntityResourceTest return entity; } + @SuppressWarnings("unchecked") @Override public void assertFieldChange(String fieldName, Object expected, Object actual) { if (expected != null && actual != null) {