diff --git a/internal/server/v1beta1/search.go b/internal/server/v1beta1/search.go index 1c33c8ca..ec208840 100644 --- 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 index 25176b3b..414de612 100644 --- 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 index cff8ff45..11edf985 100644 --- a/internal/store/elasticsearch/discovery_search_repository.go +++ b/internal/store/elasticsearch/discovery_search_repository.go @@ -26,9 +26,6 @@ var returnedAssetFieldsResult = []string{"id", "urn", "type", "service", "name", // 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")} - } maxResults := cfg.MaxResults if maxResults <= 0 { maxResults = defaultMaxResults @@ -233,6 +230,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` @@ -305,12 +306,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 index a59b37cb..b6ec86fb 100644 --- a/internal/store/elasticsearch/discovery_search_repository_test.go +++ b/internal/store/elasticsearch/discovery_search_repository_test.go @@ -3,6 +3,7 @@ package elasticsearch_test import ( "context" "encoding/json" + "fmt" "os" "sort" "testing" @@ -21,23 +22,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() @@ -67,6 +51,38 @@ func TestSearcherSearch(t *testing.T) { } tests := []searchTest{ + { + Description: "should fetch assets when text is empty", + Config: asset.SearchConfig{ + Text: "", + Filters: map[string][]string{ + "service": {"bigquery"}, + }, + }, + Expected: []expectedRow{ + {Type: "table", AssetID: "tablename-1"}, + {Type: "table", AssetID: "tablename-common"}, + {Type: "table", AssetID: "tablename-abc-common-test"}, + {Type: "table", AssetID: "tablename-mid"}, + {Type: "table", AssetID: "abc-tablename-mid"}, + {Type: "table", AssetID: "test"}, + }, + }, + { + Description: "should fetch assets when text is empty and rank by RankBy: data.profile.usage_count", + Config: asset.SearchConfig{ + Text: "", + RankBy: "data.profile.usage_count", + }, + Expected: []expectedRow{ + {Type: "table", AssetID: "tablename-common"}, + {Type: "table", AssetID: "tablename-mid"}, + {Type: "table", AssetID: "test"}, + {Type: "table", AssetID: "tablename-1"}, + {Type: "table", AssetID: "tablename-abc-common-test"}, + {Type: "table", AssetID: "abc-tablename-mid"}, + }, + }, { Description: "should fetch assets which has text in any of its fields", Config: asset.SearchConfig{ @@ -299,6 +315,8 @@ func TestSearcherSearch(t *testing.T) { results, err := repo.Search(ctx, test.Config) require.NoError(t, err) require.Equal(t, len(test.Expected), len(results)) + fmt.Println(test.Expected) + fmt.Println(results) for i, res := range test.Expected { assert.Equal(t, res.Type, results[i].Type) assert.Equal(t, res.AssetID, results[i].ID)