feat: enhance asset filters in GetAllAssets

* Support having `OR` clause in the nested data fields filters for the same key with multiple values
raystack · Jan 18, 2023 · fd2f02c · fd2f02c
1 parent a587fff
commit fd2f02c
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 41 deletions.
diff --git a/core/asset/filter.go b/core/asset/filter.go
@@ -15,7 +15,7 @@ type Filter struct {
 	SortDirection string `validate:"omitempty,oneof=asc desc"`
 	QueryFields   []string
 	Query         string
-	Data          map[string]string
+	Data          map[string][]string
 }
 
 func (f *Filter) Validate() error {
@@ -90,7 +90,13 @@ func (fb *filterBuilder) Build() (Filter, error) {
 		SortBy:        fb.sortBy,
 		SortDirection: fb.sortDirection,
 		Query:         fb.q,
-		Data:          fb.data,
+	}
+
+	if len(fb.data) != 0 {
+		flt.Data = make(map[string][]string)
+		for k, v := range fb.data {
+			flt.Data[k] = strings.Split(v, ",")
+		}
 	}
 
 	if fb.types != "" {

diff --git a/internal/server/v1beta1/asset_test.go b/internal/server/v1beta1/asset_test.go
@@ -87,9 +87,9 @@ func TestGetAllAssets(t *testing.T) {
 					SortDirection: "asc",
 					QueryFields:   []string{"name", "urn"},
 					Query:         "internal",
-					Data: map[string]string{
-						"dataset": "booking",
-						"project": "p-godata-id",
+					Data: map[string][]string{
+						"dataset": {"booking"},
+						"project": {"p-godata-id"},
 					},
 				}
 				as.EXPECT().GetAllAssets(ctx, cfg, false).Return([]asset.Asset{}, 0, nil)

diff --git a/internal/store/postgres/asset_repository.go b/internal/store/postgres/asset_repository.go
@@ -828,8 +828,8 @@ func (r *AssetRepository) BuildFilterQuery(builder sq.SelectBuilder, flt asset.F
 	}
 
 	if len(flt.Data) > 0 {
-		for key, val := range flt.Data {
-			if val == "_nonempty" {
+		for key, vals := range flt.Data {
+			if len(vals) == 1 && vals[0] == "_nonempty" {
 				field := r.buildDataField(key, true)
 				whereClause := sq.And{
 					sq.NotEq{field: nil},    // IS NOT NULL (field exists)
@@ -840,8 +840,13 @@ func (r *AssetRepository) BuildFilterQuery(builder sq.SelectBuilder, flt asset.F
 				}
 				builder = builder.Where(whereClause)
 			} else {
-				finalQuery := r.buildDataField(key, false)
-				builder = builder.Where(fmt.Sprintf("%s = '%s'", finalQuery, val))
+				dataOrClause := sq.Or{}
+				for _, v := range vals {
+					finalQuery := r.buildDataField(key, false)
+					dataOrClause = append(dataOrClause, sq.Eq{finalQuery: v})
+				}
+
+				builder = builder.Where(dataOrClause)
 			}
 		}
 	}

diff --git a/internal/store/postgres/asset_repository_test.go b/internal/store/postgres/asset_repository_test.go
@@ -182,20 +182,28 @@ func (r *AssetRepositoryTestSuite) TestBuildFilterQuery() {
 			// inconsistent test.
 			description: "should return sql query with asset's data fields filter",
 			config: asset.Filter{
-				Data: map[string]string{
-					"entity": "odpf",
+				Data: map[string][]string{
+					"entity": {"odpf"},
 				},
 			},
-			expectedQuery: `data->>'entity' = 'odpf'`,
+			expectedQuery: `(data->>'entity' = $1)`,
 		},
 		{
 			description: "should return sql query with asset's nested data fields filter",
 			config: asset.Filter{
-				Data: map[string]string{
-					"landscape.properties.project-id": "compass_001",
+				Data: map[string][]string{
+					"landscape.properties.project-id": {"compass_001"},
 				},
 			},
-			expectedQuery: `data->'landscape'->'properties'->>'project-id' = 'compass_001'`,
+			expectedQuery: `(data->'landscape'->'properties'->>'project-id' = $1)`,
+		},
+		{
+			description: "should return sql query with asset's nested multiple data fields filter ",
+			config: asset.Filter{
+				Data: map[string][]string{
+					"properties.attributes.entity": {"alpha", "beta"},
+				}},
+			expectedQuery: `(data->'properties'->'attributes'->>'entity' = $1 OR data->'properties'->'attributes'->>'entity' = $2)`,
 		},
 	}
 
@@ -328,9 +336,9 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
 
 	r.Run("should filter using asset's data fields", func() {
 		results, err := r.repository.GetAll(r.ctx, asset.Filter{
-			Data: map[string]string{
-				"entity":  "odpf",
-				"country": "th",
+			Data: map[string][]string{
+				"entity":  {"odpf"},
+				"country": {"th"},
 			},
 		})
 		r.Require().NoError(err)
@@ -344,9 +352,9 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
 
 	r.Run("should filter using asset's nested data fields", func() {
 		results, err := r.repository.GetAll(r.ctx, asset.Filter{
-			Data: map[string]string{
-				"landscape.properties.project-id": "compass_001",
-				"country":                         "vn",
+			Data: map[string][]string{
+				"landscape.properties.project-id": {"compass_001"},
+				"country":                         {"vn"},
 			},
 		})
 		r.Require().NoError(err)
@@ -360,8 +368,8 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
 
 	r.Run("should filter using asset's nonempty data fields", func() {
 		results, err := r.repository.GetAll(r.ctx, asset.Filter{
-			Data: map[string]string{
-				"properties.dependencies": "_nonempty",
+			Data: map[string][]string{
+				"properties.dependencies": {"_nonempty"},
 			},
 		})
 		r.Require().NoError(err)
@@ -375,11 +383,11 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
 
 	r.Run("should filter using asset's different nonempty data fields", func() {
 		results, err := r.repository.GetAll(r.ctx, asset.Filter{
-			Data: map[string]string{
-				"properties.dependencies": "_nonempty",
-				"entity":                  "odpf",
-				"urn":                     "j-xcvcx",
-				"country":                 "vn",
+			Data: map[string][]string{
+				"properties.dependencies": {"_nonempty"},
+				"entity":                  {"odpf"},
+				"urn":                     {"j-xcvcx"},
+				"country":                 {"vn"},
 			},
 		})
 		r.Require().NoError(err)
@@ -463,9 +471,9 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
 		{
 			Description: "should filter using asset's data fields",
 			Filter: asset.Filter{
-				Data: map[string]string{
-					"entity":  "odpf",
-					"country": "th",
+				Data: map[string][]string{
+					"entity":  {"odpf"},
+					"country": {"th"},
 				},
 			},
 			Expected: map[asset.Type]int{
@@ -477,9 +485,9 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
 		{
 			Description: "should filter using asset's nested data fields",
 			Filter: asset.Filter{
-				Data: map[string]string{
-					"landscape.properties.project-id": "compass_001",
-					"country":                         "vn",
+				Data: map[string][]string{
+					"landscape.properties.project-id": {"compass_001"},
+					"country":                         {"vn"},
 				},
 			},
 			Expected: map[asset.Type]int{
@@ -489,8 +497,8 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
 		{
 			Description: "should filter using asset's nonempty data fields",
 			Filter: asset.Filter{
-				Data: map[string]string{
-					"properties.dependencies": "_nonempty",
+				Data: map[string][]string{
+					"properties.dependencies": {"_nonempty"},
 				},
 			},
 			Expected: map[asset.Type]int{
@@ -500,11 +508,11 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
 		{
 			Description: "should filter using asset's different nonempty data fields",
 			Filter: asset.Filter{
-				Data: map[string]string{
-					"properties.dependencies": "_nonempty",
-					"entity":                  "odpf",
-					"urn":                     "j-xcvcx",
-					"country":                 "vn",
+				Data: map[string][]string{
+					"properties.dependencies": {"_nonempty"},
+					"entity":                  {"odpf"},
+					"urn":                     {"j-xcvcx"},
+					"country":                 {"vn"},
 				},
 			},
 			Expected: map[asset.Type]int{