From 370275004082a15cfde6bf8f08eed5a23faf1ef5 Mon Sep 17 00:00:00 2001
From: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com>
Date: Fri, 20 Sep 2024 11:41:13 +0530
Subject: [PATCH] fix owners patch issue (#17900)

* fix owners patch issue

* rename func

* typo

* Add Test Case

* Fix Tests

---------

Co-authored-by: Sriharsha Chintalapani <harshach@users.noreply.github.com>
Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>
(cherry picked from commit f4ce0e8693c2dad95e7f95edc457ac8a6e72d033)
---
 .../service/jdbi3/EntityRepository.java       | 21 +++++++------
 .../service/resources/EntityResourceTest.java | 31 +++++++++++++++++++
 2 files changed, 42 insertions(+), 10 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 af08270ec60..947d1b49af9 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
@@ -962,7 +962,10 @@ public abstract class EntityRepository<T extends EntityInterface> {
     updated.setUpdatedAt(System.currentTimeMillis());
 
     prepareInternal(updated, true);
-    populateOwners(updated.getOwners());
+    // Validate and populate owners
+    List<EntityReference> validatedOwners = getValidatedOwners(updated.getOwners());
+    updated.setOwners(validatedOwners);
+
     restorePatchAttributes(original, updated);
 
     // Update the attributes and relationships of an entity
@@ -994,7 +997,9 @@ public abstract class EntityRepository<T extends EntityInterface> {
     updated.setUpdatedAt(System.currentTimeMillis());
 
     prepareInternal(updated, true);
-    populateOwners(updated.getOwners());
+    // Validate and populate owners
+    List<EntityReference> validatedOwners = getValidatedOwners(updated.getOwners());
+    updated.setOwners(validatedOwners);
     restorePatchAttributes(original, updated);
 
     // Update the attributes and relationships of an entity
@@ -2000,21 +2005,17 @@ public abstract class EntityRepository<T extends EntityInterface> {
     }
   }
 
-  protected void populateOwners(List<EntityReference> owners) {
+  protected List<EntityReference> getValidatedOwners(List<EntityReference> owners) {
     if (nullOrEmpty(owners)) {
-      return;
+      return owners;
     }
     // populate owner entityRefs with all fields
     List<EntityReference> refs = validateOwners(owners);
     if (nullOrEmpty(refs)) {
-      return;
+      return owners;
     }
     refs.sort(Comparator.comparing(EntityReference::getName));
-    owners.sort(Comparator.comparing(EntityReference::getName));
-
-    for (int i = 0; i < owners.size(); i++) {
-      EntityUtil.copy(refs.get(i), owners.get(i));
-    }
+    return refs;
   }
 
   @Transaction
diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java
index b65697eed28..842ad0ef01e 100644
--- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java
+++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java
@@ -1556,6 +1556,37 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
         CatalogExceptionMessage.invalidOwnerType(TEST_DEFINITION));
   }
 
+  @Test
+  @Execution(ExecutionMode.CONCURRENT)
+  void patch_entityUpdateOwnerFromNull_200(TestInfo test) throws IOException {
+    if (!supportsOwners || !supportsPatch) {
+      return; // Entity doesn't support ownership
+    }
+
+    // Create Entity with Null Owner
+    K request = createRequest(getEntityName(test), "description", "displayName", null);
+    T createdEntity = createAndCheckEntity(request, ADMIN_AUTH_HEADERS);
+    T entity = getEntity(createdEntity.getId(), allFields, ADMIN_AUTH_HEADERS);
+
+    List<EntityReference> previousOwners = entity.getOwners();
+    if (nullOrEmpty(previousOwners)) {
+      entity.setOwners(null);
+    }
+
+    // Check if the Owner is update to user1 and user 2
+    List<EntityReference> updateOwners =
+        List.of(
+            new EntityReference().withId(USER1.getId()).withType(USER),
+            new EntityReference().withId(USER2.getId()).withType(USER));
+
+    String json = JsonUtils.pojoToJson(entity);
+    entity.setOwners(updateOwners);
+    ChangeDescription change = getChangeDescription(entity, MINOR_UPDATE);
+    fieldAdded(change, FIELD_OWNERS, updateOwners);
+    entity = patchEntityAndCheck(entity, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
+    assertEntityReferences(updateOwners, entity.getOwners());
+  }
+
   @Test
   @Execution(ExecutionMode.CONCURRENT)
   void put_entityUpdate_as_non_owner_4xx(TestInfo test) throws IOException {