Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for OR and NOT filters #76

Merged
merged 2 commits into from
Mar 30, 2023
Merged

Conversation

cmurphy
Copy link
Contributor

@cmurphy cmurphy commented Feb 16, 2023

rancher/rancher#40141

OR filters

Currently, multiple filters can be appended on the query string, and each subsequent filter is ANDed with the set. Items that pass through the filter set must match every filter in the set.

This change adds support for OR filters. A single filter key can specify multiple filters, separated by ','. An item that passes this filter can match any filter in the set.

For example, this filter matches items that have either "name" or "namespace" that match "example":

?filter=metadata.name=example,metadata.namespace=example

This filter matches items that have "name" that matches either "foo" or "bar":

?filter=metadata.name=foo,metadata.name=bar

Specifying more than one filter key in the query still ANDs each inner filter set together. This set of filters can match either a name of "foo" or "bar", but must in all cases match namespace "abc":

?filter=metadata.name=foo,metadata.name=bar&filter=metadata.namespace=abc

NOT filters

This change adds support for excluding results using the != operator.

Example to exclude all results with name "example":

?filter=metadata.name!=example

Include all results from namespace "example" but exclude those with name
"example":

?filter=metadata.namespace=example&metadata.name!=example

Exclude results with name "foo" OR exclude results with name "bar"
(effectively includes results of both types):

?filter=metadata.name!=foo,metadata.name!=bar

@cmurphy cmurphy changed the title [WIP] Add support for OR filters [WIP] Add support for OR and NOT filters Mar 7, 2023
@cmurphy cmurphy changed the title [WIP] Add support for OR and NOT filters Add support for OR and NOT filters Mar 7, 2023
Comment on lines 68 to 79
func (f OrFilter) String() string {
fields := ""
for i, field := range f.filters {
fields += strings.Join(field.field, ".")
fields += "=" + field.match
if i < len(f.filters)-1 {
fields += ","
}
}
return fields
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a string builder is more efficient then concatenations.

Suggested change
func (f OrFilter) String() string {
fields := ""
for i, field := range f.filters {
fields += strings.Join(field.field, ".")
fields += "=" + field.match
if i < len(f.filters)-1 {
fields += ","
}
}
return fields
}
func (f OrFilter) String() string {
fields := strings.Builder{}
for i, field := range f.filters {
fields.WriteString(strings.Join(field.field, "."))
fields.WriteByte('=')
fileds.WriteString(field.match)
if i < len(f.filters)-1 {
fields.WriteByte(',')
}
}
return fields.String()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines 161 to 168
fieldI, fieldJ := "", ""
for _, f := range filterOpts[i].filters {
fieldI += strings.Join(f.field, ".")
}
for _, f := range filterOpts[j].filters {
fieldJ += strings.Join(f.field, ".")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use strings.Builder

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +63 to +65
type OrFilter struct {
filters []Filter
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This doesn't have to be a struct, but this is just stylistic so you can ignore.

Suggested change
type OrFilter struct {
filters []Filter
}
type OrFilter []Filter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it makes more sense to think of the OrFilter as a container for one or more Filters, rather than as an alias for a list of filters, but you're right it would be the same.

Currently, multiple filters can be appended on the query string, and
each subsequent filter is ANDed with the set. Items that pass through
the filter set must match every filter in the set.

This change adds support for OR filters. A single filter key can specify
multiple filters, separated by ','. An item that passes this filter can
match any filter in the set.

For example, this filter matches items that have either "name" or
"namespace" that match "example":

?filter=metadata.name=example,metadata.namespace=example

This filter matches items that have "name" that matches either "foo" or
"bar":

?filter=metadata.name=foo,metadata.name=bar

Specifying more than one filter key in the query still ANDs each inner
filter set together. This set of filters can match either a name of
"foo" or "bar", but must in all cases match namespace "abc":

?filter=metadata.name=foo,metadata.name=bar&filter=metadata.namespace=abc
This change adds support for excluding results using the != operator.

Example to exclude all results with name "example":

?filter=metadata.name!=example

Include all results from namespace "example" but exclude those with name
"example":

?filter=metadata.namespace=example&metadata.name!=example

Exclude results with name "foo" OR exclude results with name "bar"
(effectively includes results of both types):

?filter=metadata.name!=foo,metadata.name!=bar
Copy link
Contributor

@rmweir rmweir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cmurphy cmurphy merged commit e6a8019 into rancher:master Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants