Fix : Propagation of owners and domain in search (#17400)

* Fix : Propagation of owners and domain in search

* update test

* optimize ADD_REMOVE_OWNERS_SCRIPT update query es script

* optimizes script by checking for inherited fields(inherited = true) before updating or adding them.

* fix glossary verion spec by adding slow since, test contains steps

* added slow in test as they are exceding in the runs

---------

Co-authored-by: Ashish Gupta <ashish@getcollate.io>

openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java

@@ -42,16 +42,30 @@ public interface SearchClient {
   String TAG_SEARCH_INDEX = "tag_search_index";
   String DEFAULT_UPDATE_SCRIPT = "for (k in params.keySet()) { ctx._source.put(k, params.get(k)) }";
   String REMOVE_DOMAINS_CHILDREN_SCRIPT = "ctx._source.remove('domain')";
+
+  // Updates field if null or if inherited is true and the parent is the same (matched by previous
+  // ID), setting inherited=true on the new object.
   String PROPAGATE_ENTITY_REFERENCE_FIELD_SCRIPT =
-      "if(ctx._source.%s == null){ ctx._source.put('%s', params)}";
+      "if (ctx._source.%s == null || (ctx._source.%s != null && ctx._source.%s.inherited == true)) { "
+          + "def newObject = params.%s; "
+          + "newObject.inherited = true; "
+          + "ctx._source.put('%s', newObject); "
+          + "}";
 
   String PROPAGATE_FIELD_SCRIPT = "ctx._source.put('%s', '%s')";
 
   String REMOVE_PROPAGATED_ENTITY_REFERENCE_FIELD_SCRIPT =
-      "if((ctx._source.%s != null) && (ctx._source.%s.id == '%s')){ ctx._source.remove('%s')}";
+      "if ((ctx._source.%s != null) && (ctx._source.%s.inherited == true)){ ctx._source.remove('%s');}";
   String REMOVE_PROPAGATED_FIELD_SCRIPT = "ctx._source.remove('%s')";
+
+  // Updates field if inherited is true and the parent is the same (matched by previous ID), setting
+  // inherited=true on the new object.
   String UPDATE_PROPAGATED_ENTITY_REFERENCE_FIELD_SCRIPT =
-      "if((ctx._source.%s == null) || (ctx._source.%s.id == '%s')) { ctx._source.put('%s', params)}";
+      "if (ctx._source.%s == null || (ctx._source.%s.inherited == true && ctx._source.%s.id == params.entityBeforeUpdate.id)) { "
+          + "def newObject = params.%s; "
+          + "newObject.inherited = true; "
+          + "ctx._source.put('%s', newObject); "
+          + "}";
   String SOFT_DELETE_RESTORE_SCRIPT = "ctx._source.put('deleted', '%s')";
   String REMOVE_TAGS_CHILDREN_SCRIPT =
       "for (int i = 0; i < ctx._source.tags.length; i++) { if (ctx._source.tags[i].tagFQN == params.fqn) { ctx._source.tags.remove(i) }}";
@@ -67,7 +81,16 @@ public interface SearchClient {
       "for (int i = 0; i < ctx._source.testSuites.length; i++) { if (ctx._source.testSuites[i].id == '%s') { ctx._source.testSuites.remove(i) }}";
 
   String ADD_REMOVE_OWNERS_SCRIPT =
-      "if (ctx._source.owners != null) { ctx._source.owners.clear(); } else { ctx._source.owners = []; } for (int i = 0; i < params.owners.size(); i++) { def newOwner = params.owners[i]; ctx._source.owners.add(newOwner); }";
+      "if (ctx._source.owners == null || ctx._source.owners.isEmpty() || "
+          + "(ctx._source.owners.size() > 0 && ctx._source.owners[0] != null && ctx._source.owners[0].inherited == true)) { "
+          + "ctx._source.owners = []; "
+          + "for (int i = 0; i < params.updatedOwners.size(); i++) { "
+          + "def newOwner = params.updatedOwners[i]; "
+          + "newOwner.inherited = true; "
+          + "ctx._source.owners.add(newOwner); "
+          + "} "
+          + "}";
+
   String NOT_IMPLEMENTED_ERROR_TYPE = "NOT_IMPLEMENTED";
 
   boolean isClientAvailable();

openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java

@@ -430,22 +430,32 @@ public class SearchRepository {
       ChangeDescription changeDescription, EntityInterface entity) {
     StringBuilder scriptTxt = new StringBuilder();
     Map<String, Object> fieldData = new HashMap<>();
+
     if (changeDescription != null) {
+      EntityRepository<?> entityRepository =
+          Entity.getEntityRepository(entity.getEntityReference().getType());
+      EntityInterface entityBeforeUpdate =
+          entityRepository.get(null, entity.getId(), entityRepository.getFields("*"));
+
       for (FieldChange field : changeDescription.getFieldsAdded()) {
         if (inheritableFields.contains(field.getName())) {
           try {
-            if (field.getName().equals(FIELD_OWNERS)
+            if (field.getName().equals(FIELD_OWNERS)) {
-                && entity.getEntityReference().getType().equalsIgnoreCase(Entity.TEST_CASE)) {
               List<EntityReference> inheritedOwners = entity.getOwners();
-              fieldData.put(field.getName(), inheritedOwners);
+              fieldData.put("updatedOwners", inheritedOwners);
               scriptTxt.append(ADD_REMOVE_OWNERS_SCRIPT);
             } else {
               EntityReference entityReference =
                   JsonUtils.readValue(field.getNewValue().toString(), EntityReference.class);
               scriptTxt.append(
                   String.format(
-                      PROPAGATE_ENTITY_REFERENCE_FIELD_SCRIPT, field.getName(), field.getName()));
+                      PROPAGATE_ENTITY_REFERENCE_FIELD_SCRIPT,
-              fieldData = JsonUtils.getMap(entityReference);
+                      field.getName(),
+                      field.getName(),
+                      field.getName(),
+                      field.getName(),
+                      field.getName()));
+              fieldData.put(field.getName(), entityReference);
             }
           } catch (UnhandledServerException e) {
             scriptTxt.append(
@@ -456,18 +466,20 @@ public class SearchRepository {
       for (FieldChange field : changeDescription.getFieldsUpdated()) {
         if (inheritableFields.contains(field.getName())) {
           try {
-            EntityReference oldEntityReference =
-                JsonUtils.readValue(field.getOldValue().toString(), EntityReference.class);
             EntityReference newEntityReference =
                 JsonUtils.readValue(field.getNewValue().toString(), EntityReference.class);
+            fieldData.put(
+                "entityBeforeUpdate",
+                JsonUtils.readValue(field.getOldValue().toString(), EntityReference.class));
             scriptTxt.append(
                 String.format(
                     UPDATE_PROPAGATED_ENTITY_REFERENCE_FIELD_SCRIPT,
                     field.getName(),
                     field.getName(),
-                    oldEntityReference.getId().toString(),
+                    field.getName(),
+                    field.getName(),
                     field.getName()));
-            fieldData = JsonUtils.getMap(newEntityReference);
+            fieldData.put(field.getName(), newEntityReference);
           } catch (UnhandledServerException e) {
             scriptTxt.append(
                 String.format(PROPAGATE_FIELD_SCRIPT, field.getName(), field.getNewValue()));
@@ -477,10 +489,9 @@ public class SearchRepository {
       for (FieldChange field : changeDescription.getFieldsDeleted()) {
         if (inheritableFields.contains(field.getName())) {
           try {
-            if (field.getName().equals(FIELD_OWNERS)
+            if (field.getName().equals(FIELD_OWNERS)) {
-                && entity.getEntityReference().getType().equalsIgnoreCase(Entity.TEST_CASE)) {
               List<EntityReference> inheritedOwners = entity.getOwners();
-              fieldData.put(field.getName(), inheritedOwners);
+              fieldData.put("updatedOwners", inheritedOwners);
               scriptTxt.append(ADD_REMOVE_OWNERS_SCRIPT);
             } else {
               EntityReference entityReference =
@@ -490,9 +501,8 @@ public class SearchRepository {
                       REMOVE_PROPAGATED_ENTITY_REFERENCE_FIELD_SCRIPT,
                       field.getName(),
                       field.getName(),
-                      entityReference.getId().toString(),
                       field.getName()));
-              fieldData = JsonUtils.getMap(entityReference);
+              fieldData.put(field.getName(), JsonUtils.getMap(entityReference));
             }
           } catch (UnhandledServerException e) {
             scriptTxt.append(String.format(REMOVE_PROPAGATED_FIELD_SCRIPT, field.getName()));

openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java

@@ -3858,6 +3858,76 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
     assertNull(entity.getDomain().getInherited());
   }
 
+  public void verifyOwnersInSearch(EntityReference entity, List<EntityReference> expectedOwners)
+      throws IOException {
+    RestClient searchClient = getSearchClient();
+    String entityType = entity.getType();
+    IndexMapping index = Entity.getSearchRepository().getIndexMapping(entityType);
+    Request request =
+        new Request(
+            "GET",
+            String.format(
+                "%s/_search", index.getIndexName(Entity.getSearchRepository().getClusterAlias())));
+    String query =
+        String.format(
+            "{\"size\": 100, \"query\": {\"bool\": {\"must\": [{\"term\": {\"_id\": \"%s\"}}]}}}",
+            entity.getId().toString());
+    request.setJsonEntity(query);
+    Response response = searchClient.performRequest(request);
+    String jsonString = EntityUtils.toString(response.getEntity());
+    HashMap<String, Object> map =
+        (HashMap<String, Object>) JsonUtils.readOrConvertValue(jsonString, HashMap.class);
+    LinkedHashMap<String, Object> hits = (LinkedHashMap<String, Object>) map.get("hits");
+    ArrayList<LinkedHashMap<String, Object>> hitsList =
+        (ArrayList<LinkedHashMap<String, Object>>) hits.get("hits");
+    assertEquals(expectedOwners.size(), hitsList.size());
+    LinkedHashMap<String, Object> source =
+        (LinkedHashMap<String, Object>) hitsList.get(0).get("_source");
+    List<EntityReference> owners = extractEntities(source, "owners");
+    assertOwners(expectedOwners, owners);
+  }
+
+  public void verifyDomainInSearch(EntityReference entity, EntityReference expectedDomain)
+      throws IOException {
+    RestClient searchClient = getSearchClient();
+    String entityType = entity.getType();
+    IndexMapping index = Entity.getSearchRepository().getIndexMapping(entityType);
+    Request request =
+        new Request(
+            "GET",
+            String.format(
+                "%s/_search", index.getIndexName(Entity.getSearchRepository().getClusterAlias())));
+    String query =
+        String.format(
+            "{\"size\": 100, \"query\": {\"bool\": {\"must\": [{\"term\": {\"_id\": \"%s\"}}]}}}",
+            entity.getId().toString());
+    request.setJsonEntity(query);
+    Response response = searchClient.performRequest(request);
+    String jsonString = EntityUtils.toString(response.getEntity());
+    HashMap<String, Object> map =
+        (HashMap<String, Object>) JsonUtils.readOrConvertValue(jsonString, HashMap.class);
+    LinkedHashMap<String, Object> hits = (LinkedHashMap<String, Object>) map.get("hits");
+    ArrayList<LinkedHashMap<String, Object>> hitsList =
+        (ArrayList<LinkedHashMap<String, Object>>) hits.get("hits");
+    assertEquals(1, hitsList.size());
+    LinkedHashMap<String, Object> source =
+        (LinkedHashMap<String, Object>) hitsList.get(0).get("_source");
+    EntityReference domain = JsonUtils.convertValue(source.get("domain"), EntityReference.class);
+    assertEquals(expectedDomain.getId(), domain.getId());
+  }
+
+  private List<EntityReference> extractEntities(
+      LinkedHashMap<String, Object> source, String field) {
+    List<LinkedHashMap<String, Object>> ownersList =
+        (List<LinkedHashMap<String, Object>>) source.get(field);
+    List<EntityReference> owners = new ArrayList<>();
+    for (LinkedHashMap<String, Object> ownerMap : ownersList) {
+      EntityReference owner = JsonUtils.convertValue(ownerMap, EntityReference.class);
+      owners.add(owner);
+    }
+    return owners;
+  }
+
   public static void assertLifeCycle(LifeCycle expected, LifeCycle actual) {
     if (expected == null) {
       assertNull(actual);

openmetadata-service/src/test/java/org/openmetadata/service/resources/databases/TableResourceTest.java

@@ -118,7 +118,6 @@ import org.openmetadata.schema.entity.services.DatabaseService;
 import org.openmetadata.schema.entity.teams.User;
 import org.openmetadata.schema.tests.CustomMetric;
 import org.openmetadata.schema.tests.TestCase;
-import org.openmetadata.schema.tests.TestCaseParameterValue;
 import org.openmetadata.schema.tests.TestSuite;
 import org.openmetadata.schema.type.ApiStatus;
 import org.openmetadata.schema.type.ChangeDescription;
@@ -178,6 +177,8 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
   private final TagResourceTest tagResourceTest = new TagResourceTest();
   private final DatabaseServiceResourceTest dbServiceTest = new DatabaseServiceResourceTest();
   private final DatabaseResourceTest dbTest = new DatabaseResourceTest();
+  private final TestSuiteResourceTest testSuiteResourceTest = new TestSuiteResourceTest();
+  private final TestCaseResourceTest testCaseResourceTest = new TestCaseResourceTest();
   private final DatabaseSchemaResourceTest schemaTest = new DatabaseSchemaResourceTest();
 
   public TableResourceTest() {
@@ -2165,10 +2166,8 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
   void test_ownershipInheritance(TestInfo test) throws HttpResponseException, IOException {
     // When a databaseSchema has no owner set, it inherits the ownership from database
     // When a table has no owner set, it inherits the ownership from databaseSchema
-    Database db =
+    CreateDatabase createDb = dbTest.createRequest(test).withOwners(Lists.newArrayList(USER1_REF));
-        dbTest.createEntity(
+    Database db = dbTest.createEntity(createDb, ADMIN_AUTH_HEADERS);
-            dbTest.createRequest(test).withOwners(Lists.newArrayList(USER1_REF)),
-            ADMIN_AUTH_HEADERS);
 
     // Ensure databaseSchema owner is inherited from database
     CreateDatabaseSchema createSchema =
@@ -2180,6 +2179,42 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
         createRequest(test).withDatabaseSchema(schema.getFullyQualifiedName());
     Table table = assertOwnerInheritance(createTable, USER1_REF);
 
+    // Ensure test case owner is inherited from table
+    CreateTestSuite createTestSuite =
+        testSuiteResourceTest.createRequest(table.getFullyQualifiedName());
+    TestSuite testSuite =
+        testSuiteResourceTest.createExecutableTestSuite(createTestSuite, ADMIN_AUTH_HEADERS);
+
+    CreateTestCase createTestCase =
+        testCaseResourceTest
+            .createRequest(test)
+            .withEntityLink(String.format("<#E::table::%s>", table.getFullyQualifiedName()))
+            .withTestSuite(testSuite.getFullyQualifiedName())
+            .withTestDefinition(TEST_DEFINITION2.getFullyQualifiedName());
+    TestCase testCase = testCaseResourceTest.assertOwnerInheritance(createTestCase, USER1_REF);
+
+    // Check owners properly updated in search
+    verifyOwnersInSearch(db.getEntityReference(), List.of(USER1_REF));
+    verifyOwnersInSearch(schema.getEntityReference(), List.of(USER1_REF));
+    verifyOwnersInSearch(table.getEntityReference(), List.of(USER1_REF));
+    verifyOwnersInSearch(testCase.getEntityReference(), List.of(USER1_REF));
+
+    // Update owners of database within same session
+    ChangeDescription change = getChangeDescription(db, MINOR_UPDATE);
+    fieldDeleted(change, "owners", List.of(USER1_REF));
+    fieldAdded(change, "owners", List.of(DATA_CONSUMER_REF));
+    db =
+        dbTest.updateAndCheckEntity(
+            createDb.withOwners(List.of(DATA_CONSUMER_REF)),
+            OK,
+            ADMIN_AUTH_HEADERS,
+            MINOR_UPDATE,
+            change);
+
+    // Check owners properly updated in search
+    verifyOwnersInSearch(schema.getEntityReference(), List.of(DATA_CONSUMER_REF));
+    verifyOwnersInSearch(table.getEntityReference(), List.of(DATA_CONSUMER_REF));
+    verifyOwnersInSearch(testCase.getEntityReference(), List.of(DATA_CONSUMER_REF));
     // Change the ownership of table and ensure further ingestion updates don't overwrite the
     // ownership
     assertOwnershipInheritanceOverride(table, createTable.withOwners(null), USER2_REF);
@@ -2193,10 +2228,9 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
   void test_domainInheritance(TestInfo test)
       throws HttpResponseException, IOException, InterruptedException {
     // Domain is inherited from databaseService > database > databaseSchema > table
-    DatabaseService dbService =
+    CreateDatabaseService createDbService =
-        dbServiceTest.createEntity(
+        dbServiceTest.createRequest(test).withDomain(DOMAIN.getFullyQualifiedName());
-            dbServiceTest.createRequest(test).withDomain(DOMAIN.getFullyQualifiedName()),
+    DatabaseService dbService = dbServiceTest.createEntity(createDbService, ADMIN_AUTH_HEADERS);
-            ADMIN_AUTH_HEADERS);
 
     // Ensure database domain is inherited from database service
     CreateDatabase createDb =
@@ -2215,19 +2249,43 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
     Table table = assertDomainInheritance(createTable, DOMAIN.getEntityReference());
 
     // Ensure test case domain is inherited from table
-    TestCaseResourceTest testCaseResourceTest = new TestCaseResourceTest();
+    CreateTestSuite createTestSuite =
+        testSuiteResourceTest.createRequest(table.getFullyQualifiedName());
+    TestSuite testSuite =
+        testSuiteResourceTest.createExecutableTestSuite(createTestSuite, ADMIN_AUTH_HEADERS);
+
     CreateTestCase createTestCase =
         testCaseResourceTest
             .createRequest(test)
             .withEntityLink(String.format("<#E::table::%s>", table.getFullyQualifiedName()))
-            .withTestSuite(TEST_SUITE1.getFullyQualifiedName())
+            .withTestSuite(testSuite.getFullyQualifiedName())
-            .withTestDefinition(TEST_DEFINITION3.getFullyQualifiedName())
+            .withTestDefinition(TEST_DEFINITION2.getFullyQualifiedName());
-            .withParameterValues(
-                List.of(
-                    new TestCaseParameterValue().withValue("100").withName("missingCountValue")));
     TestCase testCase =
         testCaseResourceTest.assertDomainInheritance(createTestCase, DOMAIN.getEntityReference());
 
+    // Check domain properly updated in search
+    verifyDomainInSearch(db.getEntityReference(), DOMAIN.getEntityReference());
+    verifyDomainInSearch(schema.getEntityReference(), DOMAIN.getEntityReference());
+    verifyDomainInSearch(table.getEntityReference(), DOMAIN.getEntityReference());
+    verifyDomainInSearch(testCase.getEntityReference(), DOMAIN.getEntityReference());
+
+    // Update domain of service within same session
+    ChangeDescription change = getChangeDescription(dbService, MINOR_UPDATE);
+    fieldUpdated(change, "domain", DOMAIN.getEntityReference(), DOMAIN1.getEntityReference());
+    dbService =
+        dbServiceTest.updateAndCheckEntity(
+            createDbService.withDomain(DOMAIN1.getFullyQualifiedName()),
+            OK,
+            ADMIN_AUTH_HEADERS,
+            MINOR_UPDATE,
+            change);
+
+    // Check domain properly updated in search
+    verifyDomainInSearch(db.getEntityReference(), DOMAIN1.getEntityReference());
+    verifyDomainInSearch(schema.getEntityReference(), DOMAIN1.getEntityReference());
+    verifyDomainInSearch(table.getEntityReference(), DOMAIN1.getEntityReference());
+    verifyDomainInSearch(testCase.getEntityReference(), DOMAIN1.getEntityReference());
+
     // Change the domain of table and ensure further ingestion updates don't overwrite the domain
     assertDomainInheritanceOverride(
         table, createTable.withDomain(null), SUB_DOMAIN.getEntityReference());
@@ -2343,7 +2401,7 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
     queryParams.put("limit", "100");
 
     ResultList<Table> tables = listEntities(queryParams, ADMIN_AUTH_HEADERS);
-    assertEquals(3, tables.getData().size());
+    assertEquals(4, tables.getData().size());
     assertNotNull(tables.getData().get(0).getTestSuite());
   }
 
@@ -2391,7 +2449,7 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
         createEntity(createWithEmptyColumnDescription, ADMIN_AUTH_HEADERS);
     // Create an entity with null description but with column description
     CreateTable createWithNullDescription =
-        createRequest(test, 3)
+        createRequest(test, 6)
             .withDatabaseSchema(schema.getFullyQualifiedName())
             .withDescription(null)
             .withColumns(listOf(columnWithDescription))
@@ -2421,6 +2479,7 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
     request.setJsonEntity(query);
     response = searchClient.performRequest(request);
     searchClient.close();
+    LOG.info("Response: {}", response);
 
     String jsonString = EntityUtils.toString(response.getEntity());
     HashMap<String, Object> map =

openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts

@@ -58,6 +58,8 @@ test.describe('Glossary tests', () => {
   test('Glossary & terms creation for reviewer as user', async ({
     browser,
   }) => {
+    test.slow(true);
+
     const { page, afterAction, apiContext } = await performAdminLogin(browser);
     const { page: page1, afterAction: afterActionUser1 } =
       await performUserLogin(browser, user1);
@@ -103,6 +105,8 @@ test.describe('Glossary tests', () => {
   test('Glossary & terms creation for reviewer as team', async ({
     browser,
   }) => {
+    test.slow(true);
+
     const { page, afterAction, apiContext } = await performAdminLogin(browser);
     const { page: page1, afterAction: afterActionUser1 } =
       await performUserLogin(browser, user2);
@@ -385,7 +389,7 @@ test.describe('Glossary tests', () => {
   });
 
   test('Rename Glossary Term and verify assets', async ({ browser }) => {
-    test.slow();
+    test.slow(true);
 
     const { page, afterAction, apiContext } = await performAdminLogin(browser);
     const table = new TableClass();

openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/GlossaryVersionPage.spec.ts

@@ -41,6 +41,8 @@ test.beforeEach(async ({ page }) => {
 });
 
 test('Glossary', async ({ page }) => {
+  test.slow(true);
+
   const glossary = new Glossary();
   const { afterAction, apiContext } = await getApiContext(page);
   await glossary.create(apiContext);
@@ -129,6 +131,8 @@ test('Glossary', async ({ page }) => {
 });
 
 test('GlossaryTerm', async ({ page }) => {
+  test.slow(true);
+
   const { term1, term2, cleanup } = await setupGlossaryAndTerms(page);
 
   await test.step('Version changes', async () => {