feat(discovery): add search suggestion api (#40)

* feat(discovery): add suggest API * feat(search): expose /v1/search/suggest * refactor(discovery): remove hardcoded value * feat(discovery): use completion suggester to build suggestions
raystack · Dec 16, 2021 · 9fc7637 · 9fc7637
1 parent 695a179
commit 9fc7637
Show file tree

Hide file tree

Showing 14 changed files with 391 additions and 36 deletions.
diff --git a/api/handlers/response.go b/api/handlers/response.go
@@ -39,3 +39,8 @@ type SearchResponse struct {
 	Description string            `json:"description"`
 	Labels      map[string]string `json:"labels"`
 }
+
+// SuggestResponse is a response for Suggest API
+type SuggestResponse struct {
+	Suggestions []string `json:"suggestions"`
+}
diff --git a/api/handlers/search_handler.go b/api/handlers/search_handler.go
@@ -40,18 +40,31 @@ func (handler *SearchHandler) Search(w http.ResponseWriter, r *http.Request) {
 	}
 	results, err := handler.discoveryService.Search(ctx, cfg)
 	if err != nil {
-		handler.log.Errorf("error searching records: %w", err)
+		handler.log.Errorf("error searching records: %s", err.Error())
 		writeJSONError(w, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
 		return
 	}
 
+	writeJSON(w, http.StatusOK, results)
+}
+
+func (handler *SearchHandler) Suggest(w http.ResponseWriter, r *http.Request) {
+	ctx := r.Context()
+	cfg, err := handler.buildSearchCfg(r.URL.Query())
 	if err != nil {
-		handler.log.Errorf("error mapping search results: %w", err)
+		writeJSONError(w, http.StatusBadRequest, err.Error())
+		return
+	}
+	suggestions, err := handler.discoveryService.Suggest(ctx, cfg)
+	if err != nil {
+		handler.log.Errorf("error building suggestions: %s", err.Error())
 		writeJSONError(w, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
 		return
 	}
 
-	writeJSON(w, http.StatusOK, results)
+	writeJSON(w, http.StatusOK, SuggestResponse{
+		Suggestions: suggestions,
+	})
 }
 
 func (handler *SearchHandler) buildSearchCfg(params url.Values) (cfg discovery.SearchConfig, err error) {

diff --git a/api/handlers/search_handler_test.go b/api/handlers/search_handler_test.go
@@ -20,9 +20,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestSearchHandler(t *testing.T) {
+func TestSearchHandlerSearch(t *testing.T) {
 	ctx := context.Background()
-	// todo: pass testCase to ValidateResponse
 	type testCase struct {
 		Title            string
 		ExpectStatus     int
@@ -197,3 +196,126 @@ func TestSearchHandler(t *testing.T) {
 		})
 	}
 }
+
+func TestSearchHandlerSuggest(t *testing.T) {
+	ctx := context.Background()
+	type testCase struct {
+		Title            string
+		ExpectStatus     int
+		Querystring      string
+		InitSearcher     func(testCase, *mock.RecordSearcher)
+		ValidateResponse func(testCase, io.Reader) error
+	}
+
+	var testCases = []testCase{
+		{
+			Title:            "should return HTTP 400 if 'text' parameter is empty or missing",
+			ExpectStatus:     http.StatusBadRequest,
+			ValidateResponse: func(tc testCase, body io.Reader) error { return nil },
+			Querystring:      "",
+		},
+		{
+			Title:       "should report HTTP 500 if searcher fails",
+			Querystring: "text=test",
+			InitSearcher: func(tc testCase, searcher *mock.RecordSearcher) {
+				cfg := discovery.SearchConfig{
+					Text:    "test",
+					Filters: map[string][]string{},
+				}
+				searcher.On("Suggest", ctx, cfg).Return([]string{}, fmt.Errorf("service unavailable"))
+			},
+			ExpectStatus: http.StatusInternalServerError,
+		},
+		{
+			Title:       "should pass filter to search config format",
+			Querystring: "text=resource&landscape=id,vn&filter.data.landscape=th&filter.type=topic&filter.service=kafka,rabbitmq",
+			InitSearcher: func(tc testCase, searcher *mock.RecordSearcher) {
+				cfg := discovery.SearchConfig{
+					Text:          "resource",
+					TypeWhiteList: []string{"topic"},
+					Filters: map[string][]string{
+						"service":        {"kafka", "rabbitmq"},
+						"data.landscape": {"th"},
+					},
+				}
+
+				searcher.On("Suggest", ctx, cfg).Return([]string{}, nil)
+			},
+			ValidateResponse: func(tc testCase, body io.Reader) error {
+				return nil
+			},
+		},
+		{
+			Title:       "should return suggestions",
+			Querystring: "text=test",
+			InitSearcher: func(tc testCase, searcher *mock.RecordSearcher) {
+				cfg := discovery.SearchConfig{
+					Text:    "test",
+					Filters: make(map[string][]string),
+				}
+				response := []string{
+					"test",
+					"test2",
+					"t est",
+					"t_est",
+				}
+
+				searcher.On("Suggest", ctx, cfg).Return(response, nil)
+			},
+			ValidateResponse: func(tc testCase, body io.Reader) error {
+				var response handlers.SuggestResponse
+				err := json.NewDecoder(body).Decode(&response)
+				if err != nil {
+					return fmt.Errorf("error reading response body: %v", err)
+				}
+				expectResponse := handlers.SuggestResponse{
+					Suggestions: []string{
+						"test",
+						"test2",
+						"t est",
+						"t_est",
+					},
+				}
+
+				if reflect.DeepEqual(response, expectResponse) == false {
+					return fmt.Errorf("expected handler response to be %#v, was %#v", expectResponse, response)
+				}
+				return nil
+			},
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.Title, func(t *testing.T) {
+			var (
+				recordSearcher = new(mock.RecordSearcher)
+			)
+			if testCase.InitSearcher != nil {
+				testCase.InitSearcher(testCase, recordSearcher)
+			}
+			defer recordSearcher.AssertExpectations(t)
+
+			params, err := url.ParseQuery(testCase.Querystring)
+			require.NoError(t, err)
+
+			requestURL := "/?" + params.Encode()
+			rr := httptest.NewRequest(http.MethodGet, requestURL, nil)
+			rw := httptest.NewRecorder()
+
+			service := discovery.NewService(nil, recordSearcher)
+			handler := handlers.NewSearchHandler(new(mock.Logger), service)
+			handler.Suggest(rw, rr)
+
+			expectStatus := testCase.ExpectStatus
+			if expectStatus == 0 {
+				expectStatus = http.StatusOK
+			}
+			assert.Equal(t, expectStatus, rw.Code)
+			if testCase.ValidateResponse != nil {
+				if err := testCase.ValidateResponse(testCase, rw.Body); err != nil {
+					t.Errorf("error validating handler response: %v", err)
+					return
+				}
+			}
+		})
+	}
+}
diff --git a/api/routes.go b/api/routes.go
@@ -66,6 +66,10 @@ func RegisterRoutes(router *mux.Router, config Config) {
 		Methods(http.MethodGet).
 		HandlerFunc(searchHandler.Search)
 
+	router.Path("/v1/search/suggest").
+		Methods(http.MethodGet).
+		HandlerFunc(searchHandler.Suggest)
+
 	router.PathPrefix("/v1/lineage/{type}/{id}").
 		Methods(http.MethodGet).
 		HandlerFunc(lineageHandler.GetLineage)

diff --git a/discovery/repo.go b/discovery/repo.go
@@ -46,4 +46,5 @@ type RecordRepositoryFactory interface {
 
 type RecordSearcher interface {
 	Search(ctx context.Context, cfg SearchConfig) (results []SearchResult, err error)
+	Suggest(ctx context.Context, cfg SearchConfig) (suggestions []string, err error)
 }
diff --git a/discovery/service.go b/discovery/service.go
@@ -50,3 +50,7 @@ func (s *Service) DeleteRecord(ctx context.Context, typeName string, recordURN s
 func (s *Service) Search(ctx context.Context, cfg SearchConfig) ([]SearchResult, error) {
 	return s.recordSearcher.Search(ctx, cfg)
 }
+
+func (s *Service) Suggest(ctx context.Context, cfg SearchConfig) ([]string, error) {
+	return s.recordSearcher.Suggest(ctx, cfg)
+}
diff --git a/discovery/service_test.go b/discovery/service_test.go
@@ -138,3 +138,40 @@ func TestServiceSearch(t *testing.T) {
 		assert.Equal(t, expected, actual)
 	})
 }
+
+func TestServiceSuggest(t *testing.T) {
+	ctx := context.TODO()
+	cfg := discovery.SearchConfig{
+		Text: "test",
+		Filters: map[string][]string{
+			"foo": {"bar"},
+		},
+	}
+	t.Run("should return error if searcher fails", func(t *testing.T) {
+		searcher := new(mock.RecordSearcher)
+		searcher.On("Suggest", ctx, cfg).Return([]string{}, errors.New("error"))
+		defer searcher.AssertExpectations(t)
+
+		service := discovery.NewService(nil, searcher)
+		_, err := service.Suggest(ctx, cfg)
+
+		assert.Error(t, err)
+	})
+
+	t.Run("should return records from searcher", func(t *testing.T) {
+		expected := []string{
+			"record-1",
+			"record-2",
+			"record-3",
+		}
+		searcher := new(mock.RecordSearcher)
+		searcher.On("Suggest", ctx, cfg).Return(expected, nil)
+		defer searcher.AssertExpectations(t)
+
+		service := discovery.NewService(nil, searcher)
+		actual, err := service.Suggest(ctx, cfg)
+
+		assert.NoError(t, err)
+		assert.Equal(t, expected, actual)
+	})
+}
diff --git a/lib/mock/mocks.go b/lib/mock/mocks.go
@@ -101,6 +101,11 @@ func (searcher *RecordSearcher) Search(ctx context.Context, cfg discovery.Search
 	return args.Get(0).([]discovery.SearchResult), args.Error(1)
 }
 
+func (searcher *RecordSearcher) Suggest(ctx context.Context, cfg discovery.SearchConfig) ([]string, error) {
+	args := searcher.Called(ctx, cfg)
+	return args.Get(0).([]string), args.Error(1)
+}
+
 type LineageProvider struct {
 	mock.Mock
 }

diff --git a/store/elasticsearch/es.go b/store/elasticsearch/es.go
@@ -8,14 +8,15 @@ import (
 
 	"github.com/elastic/go-elasticsearch/v7/esapi"
 	"github.com/odpf/columbus/record"
+	"github.com/olivere/elastic/v7"
 )
 
 // used as a utility for generating request payload
 // since github.com/olivere/elastic generates the
 // <Q> in {"query": <Q>}
 type searchQuery struct {
 	Query    interface{} `json:"query"`
-	MinScore interface{} `json:"min_score"`
+	MinScore float32     `json:"min_score"`
 }
 
 type searchHit struct {
@@ -28,6 +29,12 @@ type searchResponse struct {
 	Hits     struct {
 		Hits []searchHit `json:"hits"`
 	} `json:"hits"`
+	Suggest map[string][]struct {
+		Text    string                           `json:"text"`
+		Offset  int                              `json:"offset"`
+		Length  float32                          `json:"length"`
+		Options []elastic.SearchSuggestionOption `json:"options"`
+	} `json:"suggest"`
 }
 
 // extract error reason from an elasticsearch response

diff --git a/store/elasticsearch/schema.go b/store/elasticsearch/schema.go
@@ -11,10 +11,17 @@ var indexSettingsTemplate = `{
 	"settings": {
 		"analysis": {
 			"analyzer": {
-				"default": {
-					"type": "pattern",
-					"pattern": "([^\\p{L}\\d]+)|(?<=\\D)(?=\\d)|(?<=\\d)(?=\\D)|(?<=[\\p{L}&&[^\\p{Lu}]])(?=\\p{Lu})|(?<=\\p{Lu})(?=\\p{Lu}[\\p{L}&&[^\\p{Lu}]])"
+				"my_analyzer": {
+					"type": "custom",
+					"tokenizer": "my_tokenizer",
+					"filter": ["lowercase"]
 				}
+			},
+			"tokenizer": {
+			  "my_tokenizer": {
+				"type": "pattern",
+				"pattern": "([^\\p{L}\\d]+)|(?<=\\D)(?=\\d)|(?<=\\d)(?=\\D)|(?<=[\\p{L}&&[^\\p{Lu}]])(?=\\p{Lu})|(?<=\\p{Lu})(?=\\p{Lu}[\\p{L}&&[^\\p{Lu}]])"
+			  }
 			}
 		}
 	}
@@ -23,10 +30,27 @@ var indexSettingsTemplate = `{
 var typeIndexMapping = `{
 	"properties": {
 		"urn": {
-			"type": "text"
+			"type": "text",
+			"analyzer": "my_analyzer",
+			"fields": {
+				"keyword": {
+					"type": "keyword",
+					"ignore_above": 256.0
+				}
+			}
 		},
 		"name": {
-			"type": "text"
+			"type": "text",
+			"analyzer": "my_analyzer",
+			"fields": {
+				"suggest": {
+					"type": "completion"
+				},
+				"keyword": {
+					"type": "keyword",
+					"ignore_above": 256.0
+				}
+			}
 		},
 		"service": {
 			"type": "text",