Michael's feedback #2

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache.go

@@ -329,6 +329,7 @@ func (c *ResourceWatcherCache) fetchForOne(key string) (interface{}, error) {
 }
 
 // return all keys, optionally matching a given filter (repository list)
+// currently we're caching the index of a repo using the repo name as the key
 func (c *ResourceWatcherCache) listKeys(filters []string) ([]string, error) {
 	if err := c.checkInit(); err != nil {
 		return nil, err
@@ -418,6 +419,9 @@ func (c *ResourceWatcherCache) redisKeyFor(unstructuredObj map[string]interface{
 	return &s, nil
 }
 
+// this func is meant to be called to make sure cache client waits
+// for the cache to be fully initialized before attempting read the data out
+// of the cache
 func (c *ResourceWatcherCache) checkInit() error {
 	c.initMutex.Lock()
 	defer c.initMutex.Unlock()

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go

@@ -187,7 +187,7 @@ func (s *Server) GetAvailablePackageSummaries(ctx context.Context, request *core
 		return nil, err
 	}
 
-	packageSummaries, err := filterAndPaginateChartsAsSummaries(request.GetFilterOptions(), int(pageSize), pageOffset, cachedCharts)
+	packageSummaries, err := filterAndPaginateCharts(request.GetFilterOptions(), int(pageSize), pageOffset, cachedCharts)
 	if err != nil {
 		return nil, err
 	}

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server_test.go

@@ -649,7 +649,7 @@ func TestGetAvailablePackageSummaries(t *testing.T) {
 			},
 		},
 		{
-			testName: "uses a filter based on existing query text",
+			testName: "uses a filter based on existing query text (chart name)",
 			testRepos: []testRepoStruct{
 				{
 					name:      "index-with-categories-1",
@@ -680,6 +680,38 @@ func TestGetAvailablePackageSummaries(t *testing.T) {
 				},
 			},
 		},
+		{
+			testName: "uses a filter based on existing query text (chart keywords)",
+			testRepos: []testRepoStruct{
+				{
+					name:      "index-with-categories-1",
+					namespace: "default",
+					url:       "https://example.repo.com/charts",
+					index:     "testdata/index-with-categories.yaml",
+				},
+			},
+			request: &corev1.GetAvailablePackageSummariesRequest{
+				Context: &corev1.Context{Namespace: "blah"},
+				FilterOptions: &corev1.FilterOptions{
+					Query: "vascrip",
+				},
+			},
+			expectedResponse: &corev1.GetAvailablePackageSummariesResponse{
+				AvailablePackagesSummaries: []*corev1.AvailablePackageSummary{
+					{
+						DisplayName:      "ghost",
+						LatestPkgVersion: "13.0.14",
+						IconUrl:          "https://bitnami.com/assets/stacks/ghost/img/ghost-stack-220x234.png",
+						ShortDescription: "A simple, powerful publishing platform that allows you to share your stories with the world",
+						AvailablePackageRef: &corev1.AvailablePackageReference{
+							Identifier: "index-with-categories-1/ghost",
+							Context:    &corev1.Context{Namespace: "default"},
+							Plugin:     &plugins.Plugin{Name: "fluxv2.packages", Version: "v1alpha1"},
+						},
+					},
+				},
+			},
+		},
 		{
 			testName: "uses a filter based on non-existing query text",
 			testRepos: []testRepoStruct{

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/utils.go

@@ -215,8 +215,29 @@ func passesFilter(chart chart.Chart, filters *corev1.FilterOptions) bool {
 	}
 	if ok {
 		if query := filters.GetQuery(); len(query) > 0 {
-			ok = strings.Contains(chart.Name, query)
-			// TODO (gfichtenholt) possibly also chart keywords, but it has not been clarified yet
+			if strings.Contains(chart.Name, query) {
+				return true
+			}
+			if strings.Contains(chart.Description, query) {
+				return true
+			}
+			for _, keyword := range chart.Keywords {
+				if strings.Contains(keyword, query) {
+					return true
+				}
+			}
+			for _, source := range chart.Sources {
+				if strings.Contains(source, query) {
+					return true
+				}
+			}
+			for _, maintainer := range chart.Maintainers {
+				if strings.Contains(maintainer.Name, query) {
+					return true
+				}
+			}
+			// could not find a match for the query text
+			ok = false
 		}
 	}
 	return ok
@@ -240,7 +261,7 @@ func pageOffsetFromPageToken(pageToken string) (int, error) {
 	return int(offset), nil
 }
 
-func filterAndPaginateChartsAsSummaries(filters *corev1.FilterOptions, pageSize, pageOffset int, cachedCharts map[string]interface{}) ([]*corev1.AvailablePackageSummary, error) {
+func filterAndPaginateCharts(filters *corev1.FilterOptions, pageSize, pageOffset int, cachedCharts map[string]interface{}) ([]*corev1.AvailablePackageSummary, error) {
 	// this loop is here for 3 reasons:
 	// 1) to convert from []interface{} which is what the generic cache implementation
 	// returns for cache hits to a typed array object.