Skip to content

Commit

Permalink
fix: fix bug in evaluation of filters with filtersLogicalOperator=or (#…
Browse files Browse the repository at this point in the history
…2374)

Signed-off-by: jsvk <[email protected]>
  • Loading branch information
jsvk authored and whynowy committed Jan 17, 2023
1 parent eeae3da commit ff524af
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 3 deletions.
1 change: 1 addition & 0 deletions USERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Organizations below are **officially** using Argo Events. Please send a PR with

1. [3Rein](https://www.3rein.com)
1. [7shifts](https://www.7shifts.com)
1. [Adobe](https://adobe.com/)
1. [Akuity](https://akuity.io/)
1. [Alibaba Group](https://www.alibabagroup.com/)
1. [Ant Group](https://www.antgroup.com/)
Expand Down
11 changes: 8 additions & 3 deletions sensors/dependencies/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,16 @@ func filterEvent(filter *v1alpha1.EventDependencyFilter, operator v1alpha1.Logic
}

if operator == v1alpha1.OrLogicalOperator {
pass := (filter.Exprs != nil && exprFilter) ||
(filter.Data != nil && dataFilter) ||
(filter.Context != nil && ctxFilter) ||
(filter.Time != nil && timeFilter) ||
(filter.Script != "" && scriptFilter)

if len(errMessages) > 0 {
return exprFilter || dataFilter || ctxFilter || timeFilter || scriptFilter,
errors.New(strings.Join(errMessages, errMsgListSeparator))
return pass, errors.New(strings.Join(errMessages, errMsgListSeparator))
}
return exprFilter || dataFilter || ctxFilter || timeFilter || scriptFilter, nil
return pass, nil
}
return exprFilter && dataFilter && ctxFilter && timeFilter && scriptFilter, nil
}
Expand Down
63 changes: 63 additions & 0 deletions sensors/dependencies/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package dependencies

import (
"encoding/json"
"testing"
"time"

Expand Down Expand Up @@ -623,6 +624,68 @@ func TestFilter(t *testing.T) {
assert.True(t, pass)
})

t.Run("filtersLogicalOperator == 'or' with only a subset of filters specified", func(t *testing.T) {
filter := &v1alpha1.EventDependencyFilter{
Exprs: []v1alpha1.ExprFilter{
{
Expr: `A == "not-valid"`, // this will evaluate to false
Fields: []v1alpha1.PayloadField{
{
Path: "a.b",
Name: "A",
},
},
},
},
Data: []v1alpha1.DataFilter{ // these evaluate to false
{
Path: "a.d.e.f",
Type: "string",
Value: []string{"not-valid"},
},
{
Path: "a.h.i",
Type: "string",
Value: []string{"not-valid", "not-valid-2"},
},
},
}

eventDataBytes, err := json.Marshal(map[string]interface{}{
"a": map[string]interface{}{
"b": "c",
"d": map[string]interface{}{
"e": map[string]interface{}{
"f": "g",
},
},
"h": map[string]interface{}{
"i": "j",
},
},
})

assert.NoError(t, err)

// should return false because the two filters above evaluate to false
filtersLogicalOperator := v1alpha1.OrLogicalOperator

now := time.Now().UTC()
event := &v1alpha1.Event{
Context: &v1alpha1.EventContext{
Time: metav1.Time{
Time: time.Date(now.Year(), now.Month(), now.Day(), 16, 36, 34, 0, time.UTC),
},
},
Data: eventDataBytes,
}

pass, err := filterEvent(filter, filtersLogicalOperator, event)

assert.NoError(t, err)
assert.False(t, pass)
})

t.Run("test advanced logic: (A || B) || (C || D)", func(t *testing.T) {
// data filter: A || B == false
// expr filter: C || D == true
Expand Down

0 comments on commit ff524af

Please sign in to comment.