Merge pull request #12 from odpf/revamp-search-whitelist

fix empty search whitelist to not causing error

Author: StewartJingga

cmd/serve.go

@@ -56,10 +56,10 @@ func initRouter(
 	typeRepository := store.NewTypeRepository(esClient)
 	recordRepositoryFactory := store.NewRecordRepositoryFactory(esClient)
 	recordSearcher, err := store.NewSearcher(store.SearcherConfig{
-		Client:                 esClient,
-		TypeRepo:               typeRepository,
-		TypeWhiteList:          typeWhiteList(config.TypeWhiteListStr),
-		CachedTypesMapDuration: config.SearchTypesCacheDuration,
+		Client:              esClient,
+		TypeRepo:            typeRepository,
+		TypeWhiteList:       typeWhiteList(config.TypeWhiteListStr),
+		CachedTypesDuration: config.SearchTypesCacheDuration,
 	})
 	if err != nil {
 		log.Fatalf("error creating searcher: %v", err)

store/search.go

@@ -20,21 +20,22 @@ var (
 )
 
 type SearcherConfig struct {
-	Client                 *elasticsearch.Client
-	TypeRepo               models.TypeRepository
-	TypeWhiteList          []string
-	CachedTypesMapDuration int
+	Client              *elasticsearch.Client
+	TypeRepo            models.TypeRepository
+	TypeWhiteList       []string
+	CachedTypesDuration int
 }
 
 // Searcher is an implementation of models.RecordSearcher
 type Searcher struct {
-	cli                    *elasticsearch.Client
-	typeWhiteList          []string
-	typeWhiteListSet       map[string]bool
-	typeRepository         models.TypeRepository
-	cachedTypesMap         map[string]models.Type
-	cachedTypeExpiredOn    time.Time
-	cachedTypesMapDuration int
+	cli                 *elasticsearch.Client
+	typeWhiteList       []string
+	typeWhiteListSet    map[string]bool
+	typeRepository      models.TypeRepository
+	cachedTypes         []models.Type
+	cachedTypesMap      map[string]models.Type
+	cachedTypeExpiredOn time.Time
+	cachedTypesDuration int
 }
 
 // NewSearcher creates a new instance of Searcher
@@ -52,11 +53,11 @@ func NewSearcher(config SearcherConfig) (*Searcher, error) {
 	}
 
 	return &Searcher{
-		cli:                    config.Client,
-		typeWhiteList:          config.TypeWhiteList,
-		typeWhiteListSet:       whiteListSet,
-		typeRepository:         config.TypeRepo,
-		cachedTypesMapDuration: config.CachedTypesMapDuration,
+		cli:                 config.Client,
+		typeWhiteList:       config.TypeWhiteList,
+		typeWhiteListSet:    whiteListSet,
+		typeRepository:      config.TypeRepo,
+		cachedTypesDuration: config.CachedTypesDuration,
 	}, nil
 }
 
@@ -129,9 +130,14 @@ func (sr *Searcher) buildQuery(ctx context.Context, cfg models.SearchConfig, ind
 }
 
 func (sr *Searcher) buildQueriesFromIndices(ctx context.Context, indices []string, cfg models.SearchConfig) ([]elastic.Query, error) {
+	types, err := sr.mapIndicesToTypes(indices)
+	if err != nil {
+		return nil, err
+	}
+
 	var queries []elastic.Query
-	for _, index := range indices {
-		fields, err := sr.buildTypeFields(ctx, index)
+	for _, typ := range types {
+		fields, err := sr.buildTypeFields(typ)
 		if err != nil {
 			return nil, err
 		}
@@ -156,20 +162,15 @@ func (sr *Searcher) buildQueriesFromIndices(ctx context.Context, indices []strin
 					Fuzziness("AUTO"),
 			).
 			Filter(
-				elastic.NewTermQuery("_index", index),
+				elastic.NewTermQuery("_index", typ.Name),
 			)
 		queries = append(queries, query)
 	}
 
 	return queries, nil
 }
 
-func (sr *Searcher) buildTypeFields(ctx context.Context, typeName string) (fields []string, err error) {
-	resourceType, err := sr.getType(ctx, typeName)
-	if err != nil {
-		return fields, err
-	}
-
+func (sr *Searcher) buildTypeFields(resourceType models.Type) (fields []string, err error) {
 	fields = append(
 		fields,
 		fmt.Sprintf("%s^10", resourceType.Fields.ID),
@@ -217,41 +218,52 @@ func (sr *Searcher) searchIndices(localWhiteList []string) []string {
 	case hasGL || hasLL:
 		return anyValidStringSlice(localWhiteList, sr.typeWhiteList)
 	default:
-		return []string{defaultSearchIndex}
+		return []string{}
 	}
 }
 
-func (sr *Searcher) getType(ctx context.Context, typeName string) (models.Type, error) {
-	if sr.cachedTypesMap == nil || time.Now().After(sr.cachedTypeExpiredOn) {
-		typesMap, err := sr.buildTypesMap(ctx)
-		if err != nil {
-			return models.Type{}, err
-		}
-
-		sr.cachedTypesMap = typesMap
-		sr.cachedTypeExpiredOn = time.Now().Add(time.Duration(sr.cachedTypesMapDuration) * time.Second)
+func (sr *Searcher) mapIndicesToTypes(indices []string) ([]models.Type, error) {
+	types, err := sr.getTypes(context.Background())
+	if err != nil {
+		return types, err
+	}
+	if len(indices) == 0 {
+		return types, nil
 	}
 
-	resourceType, ok := sr.cachedTypesMap[typeName]
-	if !ok {
-		return models.Type{}, fmt.Errorf("type does not exist")
+	whitelistedTypes := []models.Type{}
+	for _, index := range indices {
+		typ, ok := sr.cachedTypesMap[index]
+		if ok {
+			whitelistedTypes = append(whitelistedTypes, typ)
+		}
 	}
 
-	return resourceType, nil
+	return whitelistedTypes, nil
 }
 
-func (sr *Searcher) buildTypesMap(ctx context.Context) (map[string]models.Type, error) {
-	types, err := sr.typeRepository.GetAll(ctx)
-	if err != nil {
-		return nil, err
+func (sr *Searcher) getTypes(ctx context.Context) ([]models.Type, error) {
+	if sr.cachedTypes == nil || time.Now().After(sr.cachedTypeExpiredOn) {
+		types, err := sr.typeRepository.GetAll(ctx)
+		if err != nil {
+			return nil, err
+		}
+
+		sr.cachedTypes = types
+		sr.cachedTypesMap = sr.buildTypesMap(types)
+		sr.cachedTypeExpiredOn = time.Now().Add(time.Duration(sr.cachedTypesDuration) * time.Second)
 	}
 
+	return sr.cachedTypes, nil
+}
+
+func (sr *Searcher) buildTypesMap(types []models.Type) map[string]models.Type {
 	typesMap := map[string]models.Type{}
 	for _, typ := range types {
 		typesMap[typ.Name] = typ
 	}
 
-	return typesMap, nil
+	return typesMap
 }
 
 func anyValidStringSlice(slices ...[]string) []string {

store/search_test.go

@@ -150,6 +150,36 @@ func TestSearch(t *testing.T) {
 		assert.Equal(t, subsetType, results[0].TypeName)
 	})
 
+	t.Run("should process all types when there is no whitelist", func(t *testing.T) {
+		esClient := esTestServer.NewClient()
+
+		testData := []searchTestData{
+			buildSampleSearchData("random_type_1"),
+			buildSampleSearchData("random_type_2"),
+		}
+		_, err := populateSearchData(esClient, testData)
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		searcher, err := store.NewSearcher(store.SearcherConfig{
+			Client:        esClient,
+			TypeRepo:      store.NewTypeRepository(esClient),
+			TypeWhiteList: []string{},
+		})
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		results, err := searcher.Search(ctx, models.SearchConfig{Text: "sample"})
+		if err != nil {
+			t.Errorf("Search: %v", err)
+			return
+		}
+
+		assert.Equal(t, 2, len(results))
+	})
+
 	t.Run("fixtures", func(t *testing.T) {
 		esClient := esTestServer.NewClient()