Skip to content

Commit e7f9deb

Browse files
authored
Replace filtering.Field type with maps (#678)
1 parent dfc3189 commit e7f9deb

File tree

3 files changed

+127
-164
lines changed

3 files changed

+127
-164
lines changed

server/registry/internal/storage/filtering/filtering.go

+7-12
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ const (
3333
StringMap FieldType = iota
3434
)
3535

36-
type Field struct {
37-
Name string
38-
Type FieldType
39-
}
40-
4136
type Filter struct {
4237
program cel.Program
4338
}
@@ -60,22 +55,22 @@ func (f *Filter) Matches(model map[string]interface{}) (bool, error) {
6055
return match, nil
6156
}
6257

63-
func NewFilter(filter string, fields []Field) (Filter, error) {
58+
func NewFilter(filter string, fields map[string]FieldType) (Filter, error) {
6459
if filter == "" {
6560
return Filter{}, nil
6661
}
6762

6863
declarations := make([]*exprpb.Decl, 0)
69-
for _, field := range fields {
70-
switch field.Type {
64+
for name, fieldType := range fields {
65+
switch fieldType {
7166
case String:
72-
declarations = append(declarations, decls.NewConst(field.Name, decls.String, nil))
67+
declarations = append(declarations, decls.NewConst(name, decls.String, nil))
7368
case Int:
74-
declarations = append(declarations, decls.NewConst(field.Name, decls.Int, nil))
69+
declarations = append(declarations, decls.NewConst(name, decls.Int, nil))
7570
case Timestamp:
76-
declarations = append(declarations, decls.NewConst(field.Name, decls.Timestamp, nil))
71+
declarations = append(declarations, decls.NewConst(name, decls.Timestamp, nil))
7772
case StringMap:
78-
declarations = append(declarations, decls.NewConst(field.Name, decls.NewMapType(decls.String, decls.String), nil))
73+
declarations = append(declarations, decls.NewConst(name, decls.NewMapType(decls.String, decls.String), nil))
7974
default:
8075
return Filter{}, status.Errorf(codes.InvalidArgument, "unknown filter argument type")
8176
}

server/registry/internal/storage/filtering/filtering_test.go

+36-68
Original file line numberDiff line numberDiff line change
@@ -25,32 +25,33 @@ func TestErrorConditions(t *testing.T) {
2525
tests := []struct {
2626
desc string
2727
filter string
28-
fields []Field
28+
fields map[string]FieldType
2929
model map[string]interface{}
3030
}{
3131
{
3232
desc: "bad field type",
3333
filter: `k == "match"`,
34-
fields: []Field{
35-
{
36-
Name: "k",
37-
Type: 999,
38-
},
34+
fields: map[string]FieldType{
35+
"k": 999,
36+
},
37+
},
38+
{
39+
desc: "bad field name",
40+
filter: `abc == "match"`,
41+
fields: map[string]FieldType{
42+
"k": 999,
3943
},
4044
},
4145
{
4246
desc: "bad filter",
4347
filter: `k xx "match"`,
44-
fields: []Field{},
48+
fields: map[string]FieldType{},
4549
},
4650
{
4751
desc: "bad filter result",
4852
filter: `k`,
49-
fields: []Field{
50-
{
51-
Name: "k",
52-
Type: String,
53-
},
53+
fields: map[string]FieldType{
54+
"k": String,
5455
},
5556
model: map[string]interface{}{
5657
"k": "match",
@@ -59,11 +60,8 @@ func TestErrorConditions(t *testing.T) {
5960
{
6061
desc: "bad model",
6162
filter: `k == "k"`,
62-
fields: []Field{
63-
{
64-
Name: "k",
65-
Type: String,
66-
},
63+
fields: map[string]FieldType{
64+
"k": String,
6765
},
6866
model: nil,
6967
},
@@ -88,18 +86,15 @@ func TestFilter_Matches(t *testing.T) {
8886
tests := []struct {
8987
desc string
9088
filter string
91-
fields []Field
89+
fields map[string]FieldType
9290
positive map[string]interface{}
9391
negative map[string]interface{}
9492
}{
9593
{
9694
desc: "empty",
9795
filter: ``,
98-
fields: []Field{
99-
{
100-
Name: "k",
101-
Type: String,
102-
},
96+
fields: map[string]FieldType{
97+
"k": String,
10398
},
10499
positive: map[string]interface{}{
105100
"k": "match",
@@ -109,11 +104,8 @@ func TestFilter_Matches(t *testing.T) {
109104
{
110105
desc: "equal to String",
111106
filter: `k == "match"`,
112-
fields: []Field{
113-
{
114-
Name: "k",
115-
Type: String,
116-
},
107+
fields: map[string]FieldType{
108+
"k": String,
117109
},
118110
positive: map[string]interface{}{
119111
"k": "match",
@@ -125,11 +117,8 @@ func TestFilter_Matches(t *testing.T) {
125117
{
126118
desc: "equal to Int",
127119
filter: `k == 123`,
128-
fields: []Field{
129-
{
130-
Name: "k",
131-
Type: Int,
132-
},
120+
fields: map[string]FieldType{
121+
"k": Int,
133122
},
134123
positive: map[string]interface{}{
135124
"k": 123,
@@ -141,11 +130,8 @@ func TestFilter_Matches(t *testing.T) {
141130
{
142131
desc: "less than Timestamp",
143132
filter: `k < timestamp("2021-01-01T00:00:00Z")`,
144-
fields: []Field{
145-
{
146-
Name: "k",
147-
Type: Timestamp,
148-
},
133+
fields: map[string]FieldType{
134+
"k": Timestamp,
149135
},
150136
positive: map[string]interface{}{
151137
"k": time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC),
@@ -157,11 +143,8 @@ func TestFilter_Matches(t *testing.T) {
157143
{
158144
desc: "greater than Timestamp",
159145
filter: `k > timestamp("2021-01-01T00:00:00Z")`,
160-
fields: []Field{
161-
{
162-
Name: "k",
163-
Type: Timestamp,
164-
},
146+
fields: map[string]FieldType{
147+
"k": Timestamp,
165148
},
166149
positive: map[string]interface{}{
167150
"k": time.Date(2022, time.January, 1, 0, 0, 0, 0, time.UTC),
@@ -173,11 +156,8 @@ func TestFilter_Matches(t *testing.T) {
173156
{
174157
desc: "has StringMap key",
175158
filter: `has(labels.match)`,
176-
fields: []Field{
177-
{
178-
Name: "labels",
179-
Type: StringMap,
180-
},
159+
fields: map[string]FieldType{
160+
"labels": StringMap,
181161
},
182162
positive: map[string]interface{}{
183163
"labels": map[string]string{
@@ -193,11 +173,8 @@ func TestFilter_Matches(t *testing.T) {
193173
{
194174
desc: "in StringMap keys",
195175
filter: `"match" in labels`,
196-
fields: []Field{
197-
{
198-
Name: "labels",
199-
Type: StringMap,
200-
},
176+
fields: map[string]FieldType{
177+
"labels": StringMap,
201178
},
202179
positive: map[string]interface{}{
203180
"labels": map[string]string{
@@ -213,11 +190,8 @@ func TestFilter_Matches(t *testing.T) {
213190
{
214191
desc: "equal to StringMap value",
215192
filter: `labels["k"] == "match"`,
216-
fields: []Field{
217-
{
218-
Name: "labels",
219-
Type: StringMap,
220-
},
193+
fields: map[string]FieldType{
194+
"labels": StringMap,
221195
},
222196
positive: map[string]interface{}{
223197
"labels": map[string]string{
@@ -233,11 +207,8 @@ func TestFilter_Matches(t *testing.T) {
233207
{
234208
desc: "substring of StringMap value",
235209
filter: `labels.k.contains("substring")`,
236-
fields: []Field{
237-
{
238-
Name: "labels",
239-
Type: StringMap,
240-
},
210+
fields: map[string]FieldType{
211+
"labels": StringMap,
241212
},
242213
positive: map[string]interface{}{
243214
"labels": map[string]string{
@@ -253,11 +224,8 @@ func TestFilter_Matches(t *testing.T) {
253224
{
254225
desc: "in StringMap value split",
255226
filter: `"match" in labels.k.split("_")`,
256-
fields: []Field{
257-
{
258-
Name: "labels",
259-
Type: StringMap,
260-
},
227+
fields: map[string]FieldType{
228+
"labels": StringMap,
261229
},
262230
positive: map[string]interface{}{
263231
"labels": map[string]string{

0 commit comments

Comments
 (0)