diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermResourceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermResourceIT.java index a658e296a945..978e4d401eeb 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermResourceIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermResourceIT.java @@ -22,10 +22,13 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; import org.openmetadata.it.auth.JwtAuthProvider; import org.openmetadata.it.factories.DatabaseSchemaTestFactory; import org.openmetadata.it.factories.DatabaseServiceTestFactory; import org.openmetadata.it.util.SdkClients; +import org.openmetadata.it.util.SharedResourceLocks; import org.openmetadata.it.util.TestNamespace; import org.openmetadata.schema.api.AddGlossaryToAssetsRequest; import org.openmetadata.schema.api.CreateTaskDetails; @@ -3420,6 +3423,127 @@ void test_glossaryTermSearchIndexUpdatedWhenGlossaryReviewerChanges(TestNamespac }); } + @Test + @ResourceLock(value = SharedResourceLocks.SEARCH_SETTINGS, mode = ResourceAccessMode.READ) + void test_hierarchySearchKeepsParentOfEveryMatchOnThePage(TestNamespace ns) throws Exception { + assertHierarchySearchKeepsFullPath(ns, "hier_page", 1); + } + + @Test + @ResourceLock(value = SharedResourceLocks.SEARCH_SETTINGS, mode = ResourceAccessMode.READ) + void test_hierarchySearchKeepsFullPathOfDeeplyNestedMatches(TestNamespace ns) throws Exception { + assertHierarchySearchKeepsFullPath(ns, "hier_deep", 4); + } + + // More matches than the page holds, nested under parents that don't match the search: every + // match on the page must still come back under its full path. Creates index synchronously with a + // refresh, so no wait is needed; the post_filter keeps other tests' terms out of the result. + private void assertHierarchySearchKeepsFullPath(TestNamespace ns, String prefix, int depth) + throws Exception { + OpenMetadataClient client = SdkClients.adminClient(); + // Letters that never occur in the generated names (hex + "hier_page/deep_g/l/m"), so no n-gram + // of the word can fuzzily match a parent's name and parents never compete with the matches + String word = randomWordFrom("jknoqstuvwxyz", 16); + int matchCount = 30; + int pageSize = 25; + Glossary glossary = + client + .glossaries() + .create( + new CreateGlossary() + .withName(ns.shortPrefix(prefix + "_g")) + .withDescription("Glossary for hierarchy search")); + try { + List path = new ArrayList<>(); + for (int level = 0; level < depth; level++) { + path.add( + createEntity( + new CreateGlossaryTerm() + .withName(ns.shortPrefix(prefix + "_l" + level)) + .withGlossary(glossary.getFullyQualifiedName()) + .withParent(level == 0 ? null : path.get(level - 1).getFullyQualifiedName()) + .withDescription("Parent that does not match the search word"))); + } + String deepestParent = path.get(depth - 1).getFullyQualifiedName(); + for (int i = 0; i < matchCount; i++) { + createEntity( + new CreateGlossaryTerm() + .withName(ns.shortPrefix(prefix + "_m" + i)) + .withGlossary(glossary.getFullyQualifiedName()) + .withParent(deepestParent) + .withDescription("Child matching " + word)); + } + + String ownGlossary = + "{\"query\":{\"term\":{\"glossary.name.keyword\":\"" + + glossary.getName().toLowerCase() + + "\"}}}"; + JsonNode node = + findByFqn( + searchGlossaryTerms(client, word, ownGlossary, pageSize, true), + glossary.getFullyQualifiedName()); + assertNotNull(node, "Glossary of the matches should be returned"); + for (GlossaryTerm parent : path) { + node = findByFqn(node.path("children"), parent.getFullyQualifiedName()); + assertNotNull(node, "Parent " + parent.getName() + " should be returned"); + } + assertEquals( + pageSize, + node.path("children").size(), + "Every match on the page should sit under its full path"); + } finally { + // Best-effort: a cleanup hiccup must not fail a test whose assertions passed + try { + client + .glossaries() + .delete( + glossary.getId().toString(), + java.util.Map.of("hardDelete", "true", "recursive", "true")); + } catch (Exception ignored) { + // the namespace-scoped glossary cannot affect other tests' assertions + } + } + } + + private static String randomWordFrom(String alphabet, int length) { + java.util.concurrent.ThreadLocalRandom random = + java.util.concurrent.ThreadLocalRandom.current(); + StringBuilder word = new StringBuilder(length); + for (int i = 0; i < length; i++) { + word.append(alphabet.charAt(random.nextInt(alphabet.length()))); + } + return word.toString(); + } + + private static JsonNode searchGlossaryTerms( + OpenMetadataClient client, String query, String postFilter, int size, boolean hierarchy) + throws Exception { + RequestOptions options = + RequestOptions.builder() + .queryParam("q", query) + .queryParam("index", "glossary_term_search_index") + .queryParam("from", "0") + .queryParam("size", String.valueOf(size)) + .queryParam("deleted", "false") + .queryParam("post_filter", postFilter) + .queryParam("getHierarchy", String.valueOf(hierarchy)) + .build(); + return new ObjectMapper() + .readTree( + client + .getHttpClient() + .executeForString(HttpMethod.GET, "/v1/search/query", null, options)); + } + + private static JsonNode findByFqn(JsonNode nodes, String fqn) { + for (JsonNode node : nodes) { + if (fqn.equals(node.path("fullyQualifiedName").asText())) { + return node; + } + } + return null; + } + private String getAssetCounts(OpenMetadataClient client, String parent) { RequestOptions.Builder optionsBuilder = RequestOptions.builder(); if (parent != null) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSearchManager.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSearchManager.java index 34eea727a875..884d115fcc3e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSearchManager.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchSearchManager.java @@ -1597,7 +1597,12 @@ private ElasticSearchRequestBuilder buildHierarchyQuery( // Add the original query as a should clause to preserve high scores b.should(originalQuery); // Add parent term queries as should clauses to fetch hierarchy - parentTermQueries.forEach(b::should); + if (indexName.equalsIgnoreCase(glossaryTermIndex) + || indexName.equalsIgnoreCase(glossaryTermAlias)) { + b.should(withZeroScore(parentTermQueries)); + } else { + parentTermQueries.forEach(b::should); + } if (indexName.equalsIgnoreCase(glossaryTermIndex)) { b.must(m -> m.match(ma -> ma.field("entityStatus").query("Approved"))); } @@ -1634,7 +1639,7 @@ private List buildSearchHierarchy( if (indexName.equalsIgnoreCase(glossaryTermIndex) || indexName.equalsIgnoreCase(glossaryTermAlias)) { - response = buildGlossaryTermSearchHierarchy(searchResponse); + response = buildGlossaryTermSearchHierarchy(indexName, searchResponse); } else if (indexName.equalsIgnoreCase(domainIndex) || indexName.equalsIgnoreCase(domainAlias)) { response = buildDomainSearchHierarchy(searchResponse); } @@ -1642,7 +1647,7 @@ private List buildSearchHierarchy( } private List buildGlossaryTermSearchHierarchy( - SearchResponse searchResponse) { + String indexName, SearchResponse searchResponse) { Map termMap = new LinkedHashMap<>(); Map rootTerms = new LinkedHashMap<>(); @@ -1666,6 +1671,7 @@ private List buildGlossaryTermSearchHierarchy( termMap.putIfAbsent(term.getFullyQualifiedName(), term); } + addMissingParents(indexName, termMap); termMap.putAll(rootTerms); termMap @@ -1693,6 +1699,69 @@ private List buildGlossaryTermSearchHierarchy( return new ArrayList<>(rootTerms.values()); } + // Parents only draw the tree, so they must not take page slots from real matches (the ones that + // miss the page are fetched by addMissingParents) + private static Query withZeroScore(List parentTermQueries) { + return Query.of( + q -> + q.constantScore( + c -> + c.filter(f -> f.bool(b -> b.should(parentTermQueries).minimumShouldMatch("1"))) + .boost(0f))); + } + + // A hit whose parent ranked below the page cut would otherwise be dropped from the tree + private void addMissingParents(String indexName, Map termMap) { + List missing = + termMap.keySet().stream() + .flatMap(fqn -> parentTermFqns(fqn).stream()) + .filter(fqn -> !termMap.containsKey(fqn)) + .distinct() + .map(FieldValue::of) + .toList(); + if (!missing.isEmpty()) { + for (EntityHierarchy parent : fetchApprovedTermsByFqn(indexName, missing)) { + parent.setChildren(new ArrayList<>()); + termMap.putIfAbsent(parent.getFullyQualifiedName(), parent); + } + } + } + + private List fetchApprovedTermsByFqn(String indexName, List fqns) { + Query byFqn = + Query.of(q -> q.terms(t -> t.field("fullyQualifiedName").terms(tv -> tv.value(fqns)))); + Query approved = Query.of(q -> q.match(m -> m.field("entityStatus").query("Approved"))); + try { + return client + .search( + s -> + s.index(indexName) + .size(fqns.size()) + .query(q -> q.bool(b -> b.filter(List.of(byFqn, approved)))), + JsonData.class) + .hits() + .hits() + .stream() + .filter(hit -> hit.source() != null) + .map(hit -> JsonUtils.readValue(hit.source().toJson().toString(), EntityHierarchy.class)) + .toList(); + } catch (IOException | ElasticsearchException e) { + LOG.warn("Could not fetch parent glossary terms for the search hierarchy", e); + return List.of(); + } + } + + // Term-level parents only: the glossary (single-part FQN) is the root and comes from each hit + private static List parentTermFqns(String fqn) { + List parents = new ArrayList<>(); + for (String parent = getParentFQN(fqn); + parent != null && getParentFQN(parent) != null; + parent = getParentFQN(parent)) { + parents.add(parent); + } + return parents; + } + private List buildDomainSearchHierarchy( SearchResponse searchResponse) { Map entityHierarchyMap = diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSearchManager.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSearchManager.java index 9bcacd7c178a..bda7a5ec2965 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSearchManager.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchSearchManager.java @@ -1661,7 +1661,12 @@ private OpenSearchRequestBuilder buildHierarchyQuery( // Add the original query as a should clause to preserve high scores b.should(originalQuery); // Add parent term queries as should clauses to fetch hierarchy - parentTermQueries.forEach(b::should); + if (indexName.equalsIgnoreCase(glossaryTermIndex) + || indexName.equalsIgnoreCase(glossaryTermAlias)) { + b.should(withZeroScore(parentTermQueries)); + } else { + parentTermQueries.forEach(b::should); + } if (indexName.equalsIgnoreCase(glossaryTermIndex)) { b.must( m -> @@ -1716,7 +1721,7 @@ private List buildSearchHierarchy( if (indexName.equalsIgnoreCase(glossaryTermIndex) || indexName.equalsIgnoreCase(glossaryTermAlias)) { - response = buildGlossaryTermSearchHierarchy(searchResponse); + response = buildGlossaryTermSearchHierarchy(indexName, searchResponse); } else if (indexName.equalsIgnoreCase(domainIndex) || indexName.equalsIgnoreCase(domainAlias)) { response = buildDomainSearchHierarchy(searchResponse); } @@ -1724,7 +1729,7 @@ private List buildSearchHierarchy( } private List buildGlossaryTermSearchHierarchy( - SearchResponse searchResponse) { + String indexName, SearchResponse searchResponse) { Map termMap = new LinkedHashMap<>(); Map rootTerms = new LinkedHashMap<>(); @@ -1748,6 +1753,7 @@ private List buildGlossaryTermSearchHierarchy( termMap.putIfAbsent(term.getFullyQualifiedName(), term); } + addMissingParents(indexName, termMap); termMap.putAll(rootTerms); termMap @@ -1775,6 +1781,70 @@ private List buildGlossaryTermSearchHierarchy( return new ArrayList<>(rootTerms.values()); } + // Parents only draw the tree, so they must not take page slots from real matches (the ones that + // miss the page are fetched by addMissingParents) + private static Query withZeroScore(List parentTermQueries) { + return Query.of( + q -> + q.constantScore( + c -> + c.filter(f -> f.bool(b -> b.should(parentTermQueries).minimumShouldMatch("1"))) + .boost(0f))); + } + + // A hit whose parent ranked below the page cut would otherwise be dropped from the tree + private void addMissingParents(String indexName, Map termMap) { + List missing = + termMap.keySet().stream() + .flatMap(fqn -> parentTermFqns(fqn).stream()) + .filter(fqn -> !termMap.containsKey(fqn)) + .distinct() + .map(FieldValue::of) + .toList(); + if (!missing.isEmpty()) { + for (EntityHierarchy parent : fetchApprovedTermsByFqn(indexName, missing)) { + parent.setChildren(new ArrayList<>()); + termMap.putIfAbsent(parent.getFullyQualifiedName(), parent); + } + } + } + + private List fetchApprovedTermsByFqn(String indexName, List fqns) { + Query byFqn = + Query.of(q -> q.terms(t -> t.field("fullyQualifiedName").terms(tv -> tv.value(fqns)))); + Query approved = + Query.of(q -> q.match(m -> m.field("entityStatus").query(FieldValue.of("Approved")))); + try { + return client + .search( + s -> + s.index(indexName) + .size(fqns.size()) + .query(q -> q.bool(b -> b.filter(List.of(byFqn, approved)))), + JsonData.class) + .hits() + .hits() + .stream() + .filter(hit -> hit.source() != null) + .map(hit -> JsonUtils.readValue(hit.source().toJson().toString(), EntityHierarchy.class)) + .toList(); + } catch (IOException | OpenSearchException e) { + LOG.warn("Could not fetch parent glossary terms for the search hierarchy", e); + return List.of(); + } + } + + // Term-level parents only: the glossary (single-part FQN) is the root and comes from each hit + private static List parentTermFqns(String fqn) { + List parents = new ArrayList<>(); + for (String parent = getParentFQN(fqn); + parent != null && getParentFQN(parent) != null; + parent = getParentFQN(parent)) { + parents.add(parent); + } + return parents; + } + private List buildDomainSearchHierarchy( SearchResponse searchResponse) { Map entityHierarchyMap =