fix-up queryByDefault

datahub-project · Oct 3, 2024 · ac7c9ce · ac7c9ce
1 parent 4e5356c
commit ac7c9ce
Show file tree

Hide file tree

Showing 15 changed files with 187 additions and 32 deletions.
diff --git a/metadata-io/src/main/java/com/linkedin/metadata/search/elasticsearch/query/ESSearchDAO.java b/metadata-io/src/main/java/com/linkedin/metadata/search/elasticsearch/query/ESSearchDAO.java
@@ -639,8 +639,14 @@ public ExplainResponse explain(
   private static void testLog(ObjectMapper mapper, SearchRequest searchRequest) {
     try {
       final String[] indices = searchRequest.indices();
-      log.warn(String.format("SearchRequest: %s", mapper.writerWithDefaultPrettyPrinter().writeValueAsString(indices)));
-      log.warn(String.format("SearchRequest: %s", searchRequest.source().toString()));
+      log.warn(
+          String.format(
+              "SearchRequest: %s",
+              mapper.writerWithDefaultPrettyPrinter().writeValueAsString(indices)));
+      log.warn(
+          String.format(
+              "SearchRequest: %s",
+              mapper.writeValueAsString(mapper.readTree(searchRequest.source().toString()))));
     } catch (JsonProcessingException e) {
       log.warn("Error writing test log");
     }

diff --git a/...n/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchRequestHandler.java b/...n/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchRequestHandler.java
@@ -58,6 +58,7 @@
 import java.util.stream.Stream;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.collections.CollectionUtils;
 import org.opensearch.action.search.SearchRequest;
@@ -80,7 +81,7 @@ public class SearchRequestHandler {
   private static final Map<List<EntitySpec>, SearchRequestHandler> REQUEST_HANDLER_BY_ENTITY_NAME =
       new ConcurrentHashMap<>();
   private final List<EntitySpec> entitySpecs;
-  private final Set<String> defaultQueryFieldNames;
+  @Getter private final Set<String> defaultQueryFieldNames;
   @Nonnull private final HighlightBuilder highlights;
 
   private final SearchConfiguration configs;

diff --git a/...io/src/test/java/com/linkedin/metadata/search/query/request/SearchRequestHandlerTest.java b/...io/src/test/java/com/linkedin/metadata/search/query/request/SearchRequestHandlerTest.java
@@ -1,5 +1,6 @@
 package com.linkedin.metadata.search.query.request;
 
+import static com.linkedin.datahub.graphql.resolvers.search.SearchUtils.SEARCHABLE_ENTITY_TYPES;
 import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;
 import static com.linkedin.metadata.utils.CriterionUtils.buildExistsCriterion;
 import static com.linkedin.metadata.utils.CriterionUtils.buildIsNullCriterion;
@@ -8,7 +9,10 @@
 import static org.testng.Assert.*;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.linkedin.data.template.StringArray;
+import com.linkedin.datahub.graphql.generated.EntityType;
+import com.linkedin.datahub.graphql.types.entitytype.EntityTypeMapper;
 import com.linkedin.metadata.TestEntitySpecBuilder;
 import com.linkedin.metadata.config.search.ExactMatchConfiguration;
 import com.linkedin.metadata.config.search.PartialConfiguration;
@@ -35,6 +39,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.opensearch.action.search.SearchRequest;
 import org.opensearch.index.query.BoolQueryBuilder;
 import org.opensearch.index.query.ExistsQueryBuilder;
@@ -628,6 +633,116 @@ public void testBrowsePathQueryFilter() {
     assertEquals(((ExistsQueryBuilder) mustHaveV1.must().get(0)).fieldName(), "browsePaths");
   }
 
+  @Test
+  public void testQueryByDefault() {
+    final Set<String> COMMON =
+        Set.of(
+            "container",
+            "fieldDescriptions",
+            "description",
+            "platform",
+            "fieldPaths",
+            "editedFieldGlossaryTerms",
+            "editedFieldDescriptions",
+            "fieldTags",
+            "id",
+            "editedDescription",
+            "qualifiedName",
+            "domains",
+            "platformInstance",
+            "tags",
+            "urn",
+            "customProperties",
+            "fieldGlossaryTerms",
+            "editedName",
+            "name",
+            "fieldLabels",
+            "glossaryTerms",
+            "editedFieldTags",
+            "displayName",
+            "title");
+
+    Map<EntityType, Set<String>> expectedQueryByDefault =
+        ImmutableMap.<EntityType, Set<String>>builder()
+            .put(
+                EntityType.DASHBOARD,
+                Stream.concat(COMMON.stream(), Stream.of("tool")).collect(Collectors.toSet()))
+            .put(
+                EntityType.CHART,
+                Stream.concat(COMMON.stream(), Stream.of("tool")).collect(Collectors.toSet()))
+            .put(
+                EntityType.MLMODEL,
+                Stream.concat(COMMON.stream(), Stream.of("type")).collect(Collectors.toSet()))
+            .put(
+                EntityType.MLFEATURE_TABLE,
+                Stream.concat(COMMON.stream(), Stream.of("features", "primaryKeys"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.MLFEATURE,
+                Stream.concat(COMMON.stream(), Stream.of("featureNamespace"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.MLPRIMARY_KEY,
+                Stream.concat(COMMON.stream(), Stream.of("featureNamespace"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.DATA_FLOW,
+                Stream.concat(COMMON.stream(), Stream.of("cluster", "orchestrator", "flowId"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.DATA_JOB,
+                Stream.concat(COMMON.stream(), Stream.of("jobId")).collect(Collectors.toSet()))
+            .put(
+                EntityType.GLOSSARY_TERM,
+                Stream.concat(
+                        COMMON.stream(),
+                        Stream.of("values", "parentNode", "relatedTerms", "definition"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.GLOSSARY_NODE,
+                Stream.concat(COMMON.stream(), Stream.of("definition", "parentNode"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.CORP_USER,
+                Stream.concat(
+                        COMMON.stream(), Stream.of("skills", "teams", "ldap", "fullName", "email"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.DOMAIN,
+                Stream.concat(COMMON.stream(), Stream.of("parentDomain"))
+                    .collect(Collectors.toSet()))
+            .put(
+                EntityType.SCHEMA_FIELD,
+                Stream.concat(COMMON.stream(), Stream.of("schemaFieldAliases", "parent"))
+                    .collect(Collectors.toSet()))
+            .build();
+
+    for (EntityType entityType : SEARCHABLE_ENTITY_TYPES) {
+      Set<String> expectedEntityQueryByDefault =
+          expectedQueryByDefault.getOrDefault(entityType, COMMON);
+      assertFalse(expectedEntityQueryByDefault.isEmpty());
+
+      EntitySpec entitySpec =
+          operationContext.getEntityRegistry().getEntitySpec(EntityTypeMapper.getName(entityType));
+      SearchRequestHandler handler =
+          SearchRequestHandler.getBuilder(
+              operationContext.getEntityRegistry(),
+              entitySpec,
+              testQueryConfig,
+              null,
+              QueryFilterRewriteChain.EMPTY);
+
+      Set<String> unexpected = new HashSet<>(handler.getDefaultQueryFieldNames());
+      unexpected.removeAll(expectedEntityQueryByDefault);
+
+      assertTrue(
+          unexpected.isEmpty(),
+          String.format(
+              "Consider whether these field(s) for entity %s should be included for general search. Fields: %s If yes, please update the test expectations. If no, please annotate the PDL model with \"queryByDefault\": false",
+              entityType, unexpected));
+    }
+  }
+
   private BoolQueryBuilder getQuery(final Criterion filterCriterion) {
     final Filter filter =
         new Filter()

diff --git a/metadata-models/src/main/pegasus/com/linkedin/common/DocumentationAssociation.pdl b/metadata-models/src/main/pegasus/com/linkedin/common/DocumentationAssociation.pdl
@@ -15,15 +15,18 @@ record DocumentationAssociation {
   @Searchable = {
     "/time": {
         "fieldName": "documentationAttributionDates",
-        "fieldType": "DATETIME"
+        "fieldType": "DATETIME",
+        "queryByDefault": false,
       },
     "/actor": {
         "fieldName": "documentationAttributionActors",
-        "fieldType": "URN"
+        "fieldType": "URN",
+        "queryByDefault": false,
       },
     "/source": {
         "fieldName": "documentationAttributionSources",
-        "fieldType": "URN"
+        "fieldType": "URN",
+        "queryByDefault": false,
       },
   }
   attribution: optional MetadataAttribution

diff --git a/metadata-models/src/main/pegasus/com/linkedin/common/Forms.pdl b/metadata-models/src/main/pegasus/com/linkedin/common/Forms.pdl
@@ -13,19 +13,23 @@ record Forms {
   @Searchable = {
     "/*/urn": {
       "fieldType": "URN",
-      "fieldName": "incompleteForms"
+      "fieldName": "incompleteForms",
+      "queryByDefault": false,
     },
     "/*/completedPrompts/*/id" : {
       "fieldType": "KEYWORD",
       "fieldName": "incompleteFormsCompletedPromptIds",
+      "queryByDefault": false,
     },
     "/*/incompletePrompts/*/id" : {
       "fieldType": "KEYWORD",
       "fieldName": "incompleteFormsIncompletePromptIds",
+      "queryByDefault": false,
     },
     "/*/completedPrompts/*/lastModified/time" : {
       "fieldType": "DATETIME",
       "fieldName": "incompleteFormsCompletedPromptResponseTimes",
+      "queryByDefault": false,
     }
   }
   incompleteForms: array[FormAssociation]
@@ -36,19 +40,23 @@ record Forms {
   @Searchable = {
     "/*/urn": {
       "fieldType": "URN",
-      "fieldName": "completedForms"
+      "fieldName": "completedForms",
+      "queryByDefault": false
     },
     "/*/completedPrompts/*/id" : {
       "fieldType": "KEYWORD",
       "fieldName": "completedFormsCompletedPromptIds",
+      "queryByDefault": false,
     },
     "/*/incompletePrompts/*/id" : {
       "fieldType": "KEYWORD",
       "fieldName": "completedFormsIncompletePromptIds",
+      "queryByDefault": false,
     },
     "/*/completedPrompts/*/lastModified/time" : {
       "fieldType": "DATETIME",
       "fieldName": "completedFormsCompletedPromptResponseTimes",
+      "queryByDefault": false,
     }
   }
   completedForms: array[FormAssociation]
@@ -59,7 +67,8 @@ record Forms {
   @Searchable = {
     "/*/form": {
       "fieldType": "URN",
-      "fieldName": "verifiedForms"
+      "fieldName": "verifiedForms",
+      "queryByDefault": false,
     }
   }
   verifications: array[FormVerificationAssociation] = []

diff --git a/metadata-models/src/main/pegasus/com/linkedin/common/GlossaryTermAssociation.pdl b/metadata-models/src/main/pegasus/com/linkedin/common/GlossaryTermAssociation.pdl
@@ -36,15 +36,18 @@ record GlossaryTermAssociation {
   @Searchable = {
     "/time": {
         "fieldName": "termAttributionDates",
-        "fieldType": "DATETIME"
+        "fieldType": "DATETIME",
+        "queryByDefault": false,
       },
     "/actor": {
         "fieldName": "termAttributionActors",
-        "fieldType": "URN"
+        "fieldType": "URN",
+        "queryByDefault": false,
       },
     "/source": {
         "fieldName": "termAttributionSources",
-        "fieldType": "URN"
+        "fieldType": "URN",
+        "queryByDefault": false,
       },
   }
   attribution: optional MetadataAttribution

diff --git a/metadata-models/src/main/pegasus/com/linkedin/common/IncidentsSummary.pdl b/metadata-models/src/main/pegasus/com/linkedin/common/IncidentsSummary.pdl
@@ -29,7 +29,8 @@ record IncidentsSummary {
            "fieldType": "URN",
            "fieldName": "resolvedIncidents",
            "hasValuesFieldName": "hasResolvedIncidents",
-           "numValuesFieldName": "numResolvedIncidents"
+           "numValuesFieldName": "numResolvedIncidents",
+           "queryByDefault": false,
        },
        "/*/type" : {
            "fieldType": "KEYWORD",
@@ -65,7 +66,8 @@ record IncidentsSummary {
             "fieldName": "activeIncidents",
             "hasValuesFieldName": "hasActiveIncidents",
             "numValuesFieldName": "numActiveIncidents",
-            "addHasValuesToFilters": true
+            "addHasValuesToFilters": true,
+            "queryByDefault": false,
         },
         "/*/type" : {
             "fieldType": "KEYWORD",

diff --git a/metadata-models/src/main/pegasus/com/linkedin/common/RoleAssociation.pdl b/metadata-models/src/main/pegasus/com/linkedin/common/RoleAssociation.pdl
@@ -18,7 +18,8 @@ record RoleAssociation {
     "fieldType": "URN",
     "hasValuesFieldName": "hasRoles",
     "addToFilters": true,
-    "filterNameOverride": "Role"
+    "filterNameOverride": "Role",
+    "queryByDefault": false,
   }
   urn: Urn
 }
diff --git a/metadata-models/src/main/pegasus/com/linkedin/common/SubTypes.pdl b/metadata-models/src/main/pegasus/com/linkedin/common/SubTypes.pdl
@@ -17,7 +17,7 @@ record SubTypes {
       "fieldType": "KEYWORD",
       "addToFilters": true,
       "filterNameOverride": "Sub Type",
-      "queryByDefault": true
+      "queryByDefault": false,
      }
    }
   typeNames: array[string]

diff --git a/metadata-models/src/main/pegasus/com/linkedin/common/TagAssociation.pdl b/metadata-models/src/main/pegasus/com/linkedin/common/TagAssociation.pdl
@@ -21,15 +21,18 @@ record TagAssociation {
   @Searchable = {
     "/time": {
         "fieldName": "tagAttributionDates",
-        "fieldType": "DATETIME"
+        "fieldType": "DATETIME",
+        "queryByDefault": false,
       },
     "/actor": {
         "fieldName": "tagAttributionActors",
-        "fieldType": "URN"
+        "fieldType": "URN",
+        "queryByDefault": false,
       },
     "/source": {
         "fieldName": "tagAttributionSources",
-        "fieldType": "URN"
+        "fieldType": "URN",
+        "queryByDefault": false,
       },
   }
   attribution: optional MetadataAttribution

diff --git a/metadata-models/src/main/pegasus/com/linkedin/glossary/GlossaryRelatedTerms.pdl b/metadata-models/src/main/pegasus/com/linkedin/glossary/GlossaryRelatedTerms.pdl
@@ -24,7 +24,7 @@ record GlossaryRelatedTerms {
     "/*": {
       "fieldName": "isRelatedTerms",
       "fieldType": "URN",
-      "boostScore": 2.0
+      "queryByDefault": false,
     }
   }
   isRelatedTerms: optional array[GlossaryTermUrn]
@@ -42,7 +42,7 @@ record GlossaryRelatedTerms {
     "/*": {
       "fieldName": "hasRelatedTerms",
       "fieldType": "URN",
-      "boostScore": 2.0
+      "queryByDefault": false,
     }
   }
   hasRelatedTerms: optional array[GlossaryTermUrn]

diff --git a/metadata-models/src/main/pegasus/com/linkedin/identity/CorpUserInfo.pdl b/metadata-models/src/main/pegasus/com/linkedin/identity/CorpUserInfo.pdl
@@ -62,7 +62,7 @@ record CorpUserInfo includes CustomProperties {
   @Searchable = {
     "fieldName": "managerLdap",
     "fieldType": "URN",
-    "queryByDefault": true
+    "queryByDefault": false,
   }
   managerUrn: optional CorpuserUrn