Skip to content

Commit 87ade1c

Browse files
bsushmithbsushmith
authored
feat: enhance asset filters in GetAllAssets (#199)
* Support having `OR` clause in the nested data fields filters for the same key with multiple values * These changes will not support data field values that inherently have a `,`(comma) in them but are not intended to be a slice.
1 parent a587fff commit 87ade1c

File tree

4 files changed

+60
-41
lines changed

4 files changed

+60
-41
lines changed

‎core/asset/filter.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ type Filter struct {
1515
SortDirection string `validate:"omitempty,oneof=asc desc"`
1616
QueryFields []string
1717
Query string
18-
Data map[string]string
18+
Data map[string][]string
1919
}
2020

2121
func (f *Filter) Validate() error {
@@ -90,7 +90,13 @@ func (fb *filterBuilder) Build() (Filter, error) {
9090
SortBy: fb.sortBy,
9191
SortDirection: fb.sortDirection,
9292
Query: fb.q,
93-
Data: fb.data,
93+
}
94+
95+
if len(fb.data) != 0 {
96+
flt.Data = make(map[string][]string)
97+
for k, v := range fb.data {
98+
flt.Data[k] = strings.Split(v, ",")
99+
}
94100
}
95101

96102
if fb.types != "" {

‎internal/server/v1beta1/asset_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ func TestGetAllAssets(t *testing.T) {
8787
SortDirection: "asc",
8888
QueryFields: []string{"name", "urn"},
8989
Query: "internal",
90-
Data: map[string]string{
91-
"dataset": "booking",
92-
"project": "p-godata-id",
90+
Data: map[string][]string{
91+
"dataset": {"booking"},
92+
"project": {"p-godata-id"},
9393
},
9494
}
9595
as.EXPECT().GetAllAssets(ctx, cfg, false).Return([]asset.Asset{}, 0, nil)

‎internal/store/postgres/asset_repository.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -828,8 +828,8 @@ func (r *AssetRepository) BuildFilterQuery(builder sq.SelectBuilder, flt asset.F
828828
}
829829

830830
if len(flt.Data) > 0 {
831-
for key, val := range flt.Data {
832-
if val == "_nonempty" {
831+
for key, vals := range flt.Data {
832+
if len(vals) == 1 && vals[0] == "_nonempty" {
833833
field := r.buildDataField(key, true)
834834
whereClause := sq.And{
835835
sq.NotEq{field: nil}, // IS NOT NULL (field exists)
@@ -840,8 +840,13 @@ func (r *AssetRepository) BuildFilterQuery(builder sq.SelectBuilder, flt asset.F
840840
}
841841
builder = builder.Where(whereClause)
842842
} else {
843-
finalQuery := r.buildDataField(key, false)
844-
builder = builder.Where(fmt.Sprintf("%s = '%s'", finalQuery, val))
843+
dataOrClause := sq.Or{}
844+
for _, v := range vals {
845+
finalQuery := r.buildDataField(key, false)
846+
dataOrClause = append(dataOrClause, sq.Eq{finalQuery: v})
847+
}
848+
849+
builder = builder.Where(dataOrClause)
845850
}
846851
}
847852
}

‎internal/store/postgres/asset_repository_test.go‎

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -182,20 +182,28 @@ func (r *AssetRepositoryTestSuite) TestBuildFilterQuery() {
182182
// inconsistent test.
183183
description: "should return sql query with asset's data fields filter",
184184
config: asset.Filter{
185-
Data: map[string]string{
186-
"entity": "odpf",
185+
Data: map[string][]string{
186+
"entity": {"odpf"},
187187
},
188188
},
189-
expectedQuery: `data->>'entity' = 'odpf'`,
189+
expectedQuery: `(data->>'entity' = $1)`,
190190
},
191191
{
192192
description: "should return sql query with asset's nested data fields filter",
193193
config: asset.Filter{
194-
Data: map[string]string{
195-
"landscape.properties.project-id": "compass_001",
194+
Data: map[string][]string{
195+
"landscape.properties.project-id": {"compass_001"},
196196
},
197197
},
198-
expectedQuery: `data->'landscape'->'properties'->>'project-id' = 'compass_001'`,
198+
expectedQuery: `(data->'landscape'->'properties'->>'project-id' = $1)`,
199+
},
200+
{
201+
description: "should return sql query with asset's nested multiple data fields filter ",
202+
config: asset.Filter{
203+
Data: map[string][]string{
204+
"properties.attributes.entity": {"alpha", "beta"},
205+
}},
206+
expectedQuery: `(data->'properties'->'attributes'->>'entity' = $1 OR data->'properties'->'attributes'->>'entity' = $2)`,
199207
},
200208
}
201209

@@ -328,9 +336,9 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
328336

329337
r.Run("should filter using asset's data fields", func() {
330338
results, err := r.repository.GetAll(r.ctx, asset.Filter{
331-
Data: map[string]string{
332-
"entity": "odpf",
333-
"country": "th",
339+
Data: map[string][]string{
340+
"entity": {"odpf"},
341+
"country": {"th"},
334342
},
335343
})
336344
r.Require().NoError(err)
@@ -344,9 +352,9 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
344352

345353
r.Run("should filter using asset's nested data fields", func() {
346354
results, err := r.repository.GetAll(r.ctx, asset.Filter{
347-
Data: map[string]string{
348-
"landscape.properties.project-id": "compass_001",
349-
"country": "vn",
355+
Data: map[string][]string{
356+
"landscape.properties.project-id": {"compass_001"},
357+
"country": {"vn"},
350358
},
351359
})
352360
r.Require().NoError(err)
@@ -360,8 +368,8 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
360368

361369
r.Run("should filter using asset's nonempty data fields", func() {
362370
results, err := r.repository.GetAll(r.ctx, asset.Filter{
363-
Data: map[string]string{
364-
"properties.dependencies": "_nonempty",
371+
Data: map[string][]string{
372+
"properties.dependencies": {"_nonempty"},
365373
},
366374
})
367375
r.Require().NoError(err)
@@ -375,11 +383,11 @@ func (r *AssetRepositoryTestSuite) TestGetAll() {
375383

376384
r.Run("should filter using asset's different nonempty data fields", func() {
377385
results, err := r.repository.GetAll(r.ctx, asset.Filter{
378-
Data: map[string]string{
379-
"properties.dependencies": "_nonempty",
380-
"entity": "odpf",
381-
"urn": "j-xcvcx",
382-
"country": "vn",
386+
Data: map[string][]string{
387+
"properties.dependencies": {"_nonempty"},
388+
"entity": {"odpf"},
389+
"urn": {"j-xcvcx"},
390+
"country": {"vn"},
383391
},
384392
})
385393
r.Require().NoError(err)
@@ -463,9 +471,9 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
463471
{
464472
Description: "should filter using asset's data fields",
465473
Filter: asset.Filter{
466-
Data: map[string]string{
467-
"entity": "odpf",
468-
"country": "th",
474+
Data: map[string][]string{
475+
"entity": {"odpf"},
476+
"country": {"th"},
469477
},
470478
},
471479
Expected: map[asset.Type]int{
@@ -477,9 +485,9 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
477485
{
478486
Description: "should filter using asset's nested data fields",
479487
Filter: asset.Filter{
480-
Data: map[string]string{
481-
"landscape.properties.project-id": "compass_001",
482-
"country": "vn",
488+
Data: map[string][]string{
489+
"landscape.properties.project-id": {"compass_001"},
490+
"country": {"vn"},
483491
},
484492
},
485493
Expected: map[asset.Type]int{
@@ -489,8 +497,8 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
489497
{
490498
Description: "should filter using asset's nonempty data fields",
491499
Filter: asset.Filter{
492-
Data: map[string]string{
493-
"properties.dependencies": "_nonempty",
500+
Data: map[string][]string{
501+
"properties.dependencies": {"_nonempty"},
494502
},
495503
},
496504
Expected: map[asset.Type]int{
@@ -500,11 +508,11 @@ func (r *AssetRepositoryTestSuite) TestGetTypes() {
500508
{
501509
Description: "should filter using asset's different nonempty data fields",
502510
Filter: asset.Filter{
503-
Data: map[string]string{
504-
"properties.dependencies": "_nonempty",
505-
"entity": "odpf",
506-
"urn": "j-xcvcx",
507-
"country": "vn",
511+
Data: map[string][]string{
512+
"properties.dependencies": {"_nonempty"},
513+
"entity": {"odpf"},
514+
"urn": {"j-xcvcx"},
515+
"country": {"vn"},
508516
},
509517
},
510518
Expected: map[asset.Type]int{

0 commit comments

Comments
 (0)