feat: enable empty text search (#57)

* feat: enable empty text search * add test cases --------- Co-authored-by: anjali.agarwal <[email protected]>
goto · Oct 5, 2023 · d11283b · d11283b
1 parent dc5aabe
commit d11283b
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 32 deletions.
diff --git a/internal/server/v1beta1/search.go b/internal/server/v1beta1/search.go
@@ -18,9 +18,6 @@ func (server *APIServer) SearchAssets(ctx context.Context, req *compassv1beta1.S
 	}
 
 	text := strings.TrimSpace(req.GetText())
-	if text == "" {
-		return nil, status.Error(codes.InvalidArgument, "'text' must be specified")
-	}
 
 	cfg := asset.SearchConfig{
 		Text:          text,

diff --git a/internal/server/v1beta1/search_test.go b/internal/server/v1beta1/search_test.go
@@ -34,11 +34,6 @@ func TestSearch(t *testing.T) {
 	}
 
 	testCases := []testCase{
-		{
-			Description:  "should return invalid argument if 'text' parameter is empty or missing",
-			ExpectStatus: codes.InvalidArgument,
-			Request:      &compassv1beta1.SearchAssetsRequest{},
-		},
 		{
 			Description: "should report internal server if asset searcher fails",
 			Request: &compassv1beta1.SearchAssetsRequest{

diff --git a/internal/store/elasticsearch/discovery_search_repository.go b/internal/store/elasticsearch/discovery_search_repository.go
@@ -24,11 +24,7 @@ const (
 
 // Search the asset store
 func (repo *DiscoveryRepository) Search(ctx context.Context, cfg asset.SearchConfig) (results []asset.SearchResult, err error) {
-	if strings.TrimSpace(cfg.Text) == "" {
-		return nil, asset.DiscoveryError{Op: "Search", Err: errors.New("search text cannot be empty")}
-	}
 	var returnedAssetFieldsResult []string
-
 	maxResults := cfg.MaxResults
 	if maxResults <= 0 {
 		maxResults = defaultMaxResults
@@ -242,6 +238,10 @@ func buildSuggestQuery(cfg asset.SearchConfig) (io.Reader, error) {
 }
 
 func buildTextQuery(q *elastic.BoolQuery, cfg asset.SearchConfig) {
+	if strings.TrimSpace(cfg.Text) == "" {
+		q.Should(elastic.NewMatchAllQuery())
+	}
+
 	boostedFields := []string{"urn^10", "name^5"}
 	q.Should(
 		// Phrase query cannot have `FUZZINESS`
@@ -314,12 +314,13 @@ func buildFilterExistsQueries(q *elastic.BoolQuery, fields []string) {
 
 func buildFunctionScoreQuery(query elastic.Query, rankBy, text string) elastic.Query {
 	// Added exact match term query here so that exact match gets higher priority.
-	fsQuery := elastic.NewFunctionScoreQuery().
-		Add(
+	fsQuery := elastic.NewFunctionScoreQuery()
+	if text != "" {
+		fsQuery.Add(
 			elastic.NewTermQuery("name.keyword", text),
 			elastic.NewWeightFactorFunction(2),
 		)
-
+	}
 	if rankBy != "" {
 		fsQuery.AddScoreFunc(
 			elastic.NewFieldValueFactorFunction().

diff --git a/internal/store/elasticsearch/discovery_search_repository_test.go b/internal/store/elasticsearch/discovery_search_repository_test.go
@@ -21,23 +21,6 @@ type searchTestData struct {
 
 func TestSearcherSearch(t *testing.T) {
 	ctx := context.TODO()
-	t.Run("should return an error if search string is empty", func(t *testing.T) {
-		cli, err := esTestServer.NewClient()
-		require.NoError(t, err)
-		esClient, err := store.NewClient(
-			log.NewNoop(),
-			store.Config{},
-			store.WithClient(cli),
-		)
-		require.NoError(t, err)
-
-		repo := store.NewDiscoveryRepository(esClient, log.NewNoop())
-		_, err = repo.Search(ctx, asset.SearchConfig{
-			Text: "",
-		})
-
-		assert.Error(t, err)
-	})
 
 	t.Run("fixtures", func(t *testing.T) {
 		cli, err := esTestServer.NewClient()
@@ -95,6 +78,39 @@ func TestSearcherSearch(t *testing.T) {
 					{Type: "topic", AssetID: "transaction", Service: "rabbitmq", Data: map[string]interface{}{"company": "gotocompany", "description": "This publishes all the invoices from each of invoice storage where the invoice will be filtered and checked using invoice filterer and invoice checker", "environment": "production", "partition": float64(1), "topic_name": "transaction"}},
 				},
 			},
+			{
+				Description: "should fetch assets with empty text",
+				Config: asset.SearchConfig{
+					Text:          "",
+					IncludeFields: []string{"id", "type"},
+					Filters:       map[string][]string{"service": {"bigquery"}},
+				},
+				Expected: []expectedRow{
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-1"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-common"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-abc-common-test"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-mid"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/abc-tablename-mid"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/test"},
+				},
+			},
+			{
+				Description: "should fetch assets with empty text and rank by",
+				Config: asset.SearchConfig{
+					Text:          "",
+					RankBy:        "data.profile.usage_count",
+					IncludeFields: []string{"id", "type"},
+					Filters:       map[string][]string{"service": {"bigquery"}},
+				},
+				Expected: []expectedRow{
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-common"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-mid"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/test"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-1"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/tablename-abc-common-test"},
+					{Type: "table", AssetID: "bigquery::gcpproject/dataset/abc-tablename-mid"},
+				},
+			},
 			{
 				Description: "should fetch assets which has text in any of its fields",
 				Config: asset.SearchConfig{