Skip to content

Commit

Permalink
Apply feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jpbetz committed Oct 31, 2024
1 parent a8ac1f5 commit bda634e
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 40 deletions.
10 changes: 8 additions & 2 deletions fieldpath/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func MakePrefixMatcherOrDie(parts ...interface{}) *SetMatcher {
//
// - PathElementMatcher - for wildcards, `MatchAnyPathElement()` can be used as well.
// - PathElement - for any path element
// - value.FieldList - for associative list keys
// - value.FieldList - for listMap keys
// - value.Value - for scalar list elements
// - string - For field names
// - int - for array indices
Expand All @@ -164,19 +164,25 @@ func PrefixMatcher(parts ...interface{}) (*SetMatcher, error) {
var pattern PathElementMatcher
switch t := part.(type) {
case PathElementMatcher:
// any path matcher, including wildcard
pattern = t
case PathElement:
// any path element
pattern = PathElementMatcher{PathElement: t}
case *value.FieldList:
// a listMap key
if len(*t) == 0 {
return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)")
}
pattern = PathElementMatcher{PathElement: PathElement{Key: t}}
case value.Value:
// a scalar or set-type list element
pattern = PathElementMatcher{PathElement: PathElement{Value: &t}}
case string:
// a plain field name
pattern = PathElementMatcher{PathElement: PathElement{FieldName: &t}}
case int:
// a plain list index
pattern = PathElementMatcher{PathElement: PathElement{Index: &t}}
default:
return nil, fmt.Errorf("unexpected type %T", t)
Expand Down Expand Up @@ -270,7 +276,7 @@ type SetMemberMatcher struct {
Child *SetMatcher
}

// PathElementMatcher defined a match matcher for a PathElement.
// PathElementMatcher defined a path matcher for a PathElement.
type PathElementMatcher struct {
// Wildcard indicates that all PathElements are matched by this matcher.
// If set, PathElement is ignored.
Expand Down
78 changes: 40 additions & 38 deletions fieldpath/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,16 +769,20 @@ func TestFilterByPattern(t *testing.T) {
input: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "containers"),
MakePathOrDie("spec", "containers", 0),
MakePathOrDie("spec", "containers", 0, "resources"),
MakePathOrDie("spec", "containers", 0, "resources", "limits"),
MakePathOrDie("spec", "containers", 0, "resources", "limits", "cpu"),
MakePathOrDie("spec", "containers", 0, "resources", "requests"),
MakePathOrDie("spec", "containers", 0, "resources", "requests", "cpu"),
MakePathOrDie("spec", "containers", 0, "resources", "claims"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 0),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "name"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "request"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 1),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "name"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "request"),
MakePathOrDie("spec", "containers", 1),
MakePathOrDie("spec", "containers", 1, "resources"),
MakePathOrDie("spec", "containers", 1, "resources", "limits"),
MakePathOrDie("spec", "containers", 1, "resources", "limits", "cpu"),
Expand All @@ -787,16 +791,20 @@ func TestFilterByPattern(t *testing.T) {
expect: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "containers"),
MakePathOrDie("spec", "containers", 0),
MakePathOrDie("spec", "containers", 0, "resources"),
MakePathOrDie("spec", "containers", 0, "resources", "limits"),
MakePathOrDie("spec", "containers", 0, "resources", "limits", "cpu"),
MakePathOrDie("spec", "containers", 0, "resources", "requests"),
MakePathOrDie("spec", "containers", 0, "resources", "requests", "cpu"),
MakePathOrDie("spec", "containers", 0, "resources", "claims"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 0),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "name"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "request"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 1),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "name"),
MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "request"),
MakePathOrDie("spec", "containers", 1),
MakePathOrDie("spec", "containers", 1, "resources"),
MakePathOrDie("spec", "containers", 1, "resources", "limits"),
MakePathOrDie("spec", "containers", 1, "resources", "limits", "cpu"),
Expand Down Expand Up @@ -831,6 +839,7 @@ func TestFilterByPattern(t *testing.T) {
input: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "containers"),
MakePathOrDie("spec", "containers", 0),
MakePathOrDie("spec", "containers", 0, "image"),
MakePathOrDie("spec", "containers", 0, "workingDir"),
MakePathOrDie("spec", "containers", 0, "resources"),
Expand All @@ -839,6 +848,7 @@ func TestFilterByPattern(t *testing.T) {
expect: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "containers"),
MakePathOrDie("spec", "containers", 0),
MakePathOrDie("spec", "containers", 0, "resources"),
),
},
Expand Down Expand Up @@ -880,44 +890,6 @@ func TestFilterByPattern(t *testing.T) {
}, "field"),
),
},
{
name: "filter listMap key",
input: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "listMap",
&value.FieldList{
{Name: "key1", Value: value.NewValueInterface("value1")},
{Name: "key2", Value: value.NewValueInterface("value2")},
}),
MakePathOrDie("spec", "listMap",
&value.FieldList{
{Name: "key1", Value: value.NewValueInterface("value1")},
{Name: "key2", Value: value.NewValueInterface("value2")},
}, "field"),
MakePathOrDie("spec", "listMap",
&value.FieldList{
{Name: "key1", Value: value.NewValueInterface("valueX")},
{Name: "key2", Value: value.NewValueInterface("valueY")},
}, "field"),
),
filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "listMap", &value.FieldList{
{Name: "key1", Value: value.NewValueInterface("value1")},
{Name: "key2", Value: value.NewValueInterface("value2")},
})),
expect: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "listMap",
&value.FieldList{
{Name: "key1", Value: value.NewValueInterface("value1")},
{Name: "key2", Value: value.NewValueInterface("value2")},
}),
MakePathOrDie("spec", "listMap",
&value.FieldList{
{Name: "key1", Value: value.NewValueInterface("value1")},
{Name: "key2", Value: value.NewValueInterface("value2")},
}, "field"),
),
},
{
name: "filter value",
input: NewSet(
Expand All @@ -935,55 +907,80 @@ func TestFilterByPattern(t *testing.T) {
name: "filter by index",
input: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "list"),
MakePathOrDie("spec", "list", 0),
MakePathOrDie("spec", "list", 0, "value"),
MakePathOrDie("spec", "list", 1),
MakePathOrDie("spec", "list", 1, "value"),
),
filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "list", 1, "value")),
expect: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "list"),
MakePathOrDie("spec", "list", 1),
MakePathOrDie("spec", "list", 1, "value"),
),
},
{
name: "multiple index matchers",
input: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "list"),
MakePathOrDie("spec", "list", 0),
MakePathOrDie("spec", "list", 0, "value"),
MakePathOrDie("spec", "list", 1),
MakePathOrDie("spec", "list", 1, "value"),
MakePathOrDie("spec", "list", 2),
MakePathOrDie("spec", "list", 2, "value"),
),
filter: NewIncludeMatcherFilter(
MakePrefixMatcherOrDie("spec", "list", 0, "value"),
MakePrefixMatcherOrDie("spec", "list", 1, "value"),
),
expect: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "list"),
MakePathOrDie("spec", "list", 0),
MakePathOrDie("spec", "list", 0, "value"),
MakePathOrDie("spec", "list", 1),
MakePathOrDie("spec", "list", 1, "value"),
),
},
{
name: "multiple field matchers",
input: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "f1"),
MakePathOrDie("spec", "f1", "f11"),
MakePathOrDie("spec", "f2"),
MakePathOrDie("spec", "f2", "f21"),
MakePathOrDie("spec", "f3"),
MakePathOrDie("spec", "f3", "f31"),
),
filter: NewIncludeMatcherFilter(
MakePrefixMatcherOrDie("spec", "f1"),
MakePrefixMatcherOrDie("spec", "f3"),
),
expect: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "f1"),
MakePathOrDie("spec", "f1", "f11"),
MakePathOrDie("spec", "f3"),
MakePathOrDie("spec", "f3", "f31"),
),
},
{
name: "wildcard takes precedence",
input: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "list"),
MakePathOrDie("spec", "list", 0),
MakePathOrDie("spec", "list", 0, "f1"),
MakePathOrDie("spec", "list", 0, "f2"),
MakePathOrDie("spec", "list", 1),
MakePathOrDie("spec", "list", 1, "f1"),
MakePathOrDie("spec", "list", 1, "f2"),
MakePathOrDie("spec", "list", 2),
MakePathOrDie("spec", "list", 2, "f1"),
MakePathOrDie("spec", "list", 2, "f2"),
),
Expand All @@ -992,8 +989,13 @@ func TestFilterByPattern(t *testing.T) {
MakePrefixMatcherOrDie("spec", "list", 1, "f2"), // ignored
),
expect: NewSet(
MakePathOrDie("spec"),
MakePathOrDie("spec", "list"),
MakePathOrDie("spec", "list", 0),
MakePathOrDie("spec", "list", 0, "f1"),
MakePathOrDie("spec", "list", 1),
MakePathOrDie("spec", "list", 1, "f1"),
MakePathOrDie("spec", "list", 2),
MakePathOrDie("spec", "list", 2, "f1"),
),
},
Expand Down

0 comments on commit bda634e

Please sign in to comment.