diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java index b7e8485046e..982439296d7 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchClient.java +++ b/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(); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java index 370b7c329b4..b7a9433149b 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/SearchRepository.java +++ b/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 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) - && entity.getEntityReference().getType().equalsIgnoreCase(Entity.TEST_CASE)) { + if (field.getName().equals(FIELD_OWNERS)) { List 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())); - fieldData = JsonUtils.getMap(entityReference); + PROPAGATE_ENTITY_REFERENCE_FIELD_SCRIPT, + 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) - && entity.getEntityReference().getType().equalsIgnoreCase(Entity.TEST_CASE)) { + if (field.getName().equals(FIELD_OWNERS)) { List 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())); 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 0d59f662d3b..bc5356f0777 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 @@ -3858,6 +3858,76 @@ public abstract class EntityResourceTest 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 map = + (HashMap) JsonUtils.readOrConvertValue(jsonString, HashMap.class); + LinkedHashMap hits = (LinkedHashMap) map.get("hits"); + ArrayList> hitsList = + (ArrayList>) hits.get("hits"); + assertEquals(expectedOwners.size(), hitsList.size()); + LinkedHashMap source = + (LinkedHashMap) hitsList.get(0).get("_source"); + List 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 map = + (HashMap) JsonUtils.readOrConvertValue(jsonString, HashMap.class); + LinkedHashMap hits = (LinkedHashMap) map.get("hits"); + ArrayList> hitsList = + (ArrayList>) hits.get("hits"); + assertEquals(1, hitsList.size()); + LinkedHashMap source = + (LinkedHashMap) hitsList.get(0).get("_source"); + EntityReference domain = JsonUtils.convertValue(source.get("domain"), EntityReference.class); + assertEquals(expectedDomain.getId(), domain.getId()); + } + + private List extractEntities( + LinkedHashMap source, String field) { + List> ownersList = + (List>) source.get(field); + List owners = new ArrayList<>(); + for (LinkedHashMap 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); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/databases/TableResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/databases/TableResourceTest.java index d51e7472bf6..ab3cd519e24 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/databases/TableResourceTest.java +++ b/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 { 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 { 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 = - dbTest.createEntity( - dbTest.createRequest(test).withOwners(Lists.newArrayList(USER1_REF)), - ADMIN_AUTH_HEADERS); + CreateDatabase createDb = dbTest.createRequest(test).withOwners(Lists.newArrayList(USER1_REF)); + Database db = dbTest.createEntity(createDb, ADMIN_AUTH_HEADERS); // Ensure databaseSchema owner is inherited from database CreateDatabaseSchema createSchema = @@ -2180,6 +2179,42 @@ public class TableResourceTest extends EntityResourceTest { 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 { void test_domainInheritance(TestInfo test) throws HttpResponseException, IOException, InterruptedException { // Domain is inherited from databaseService > database > databaseSchema > table - DatabaseService dbService = - dbServiceTest.createEntity( - dbServiceTest.createRequest(test).withDomain(DOMAIN.getFullyQualifiedName()), - ADMIN_AUTH_HEADERS); + CreateDatabaseService createDbService = + dbServiceTest.createRequest(test).withDomain(DOMAIN.getFullyQualifiedName()); + DatabaseService dbService = dbServiceTest.createEntity(createDbService, ADMIN_AUTH_HEADERS); // Ensure database domain is inherited from database service CreateDatabase createDb = @@ -2215,19 +2249,43 @@ public class TableResourceTest extends EntityResourceTest { 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()) - .withTestDefinition(TEST_DEFINITION3.getFullyQualifiedName()) - .withParameterValues( - List.of( - new TestCaseParameterValue().withValue("100").withName("missingCountValue"))); + .withTestSuite(testSuite.getFullyQualifiedName()) + .withTestDefinition(TEST_DEFINITION2.getFullyQualifiedName()); 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 { queryParams.put("limit", "100"); ResultList 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 { 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 { request.setJsonEntity(query); response = searchClient.performRequest(request); searchClient.close(); + LOG.info("Response: {}", response); String jsonString = EntityUtils.toString(response.getEntity()); HashMap map = diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts index 9b5bddf8db6..7d90a245d57 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts +++ b/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(); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/GlossaryVersionPage.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/GlossaryVersionPage.spec.ts index 025efe5bd44..a0ca8b73059 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/GlossaryVersionPage.spec.ts +++ b/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 () => {