Skip to content

Commit 9590fe5

Browse files
uptownhruptownhr
committed
fix: make != comparator work as NOT IN for data filters with multiple values
The != comparator in DataFilter with multiple values was exhibiting unintuitive behavior that made it practically unusable. Instead of implementing true "NOT IN" semantics (value must not match ALL items in array), it was implementing "not equal to ANY" (passes on first mismatch). This meant that with comparator: "!=" and value: ["id1", "id2", "id3"], an event with "id1" would PASS (because "id1" != "id2" is true), when it should FAIL (because "id1" IS in the array). Changes: - Modified pkg/sensors/dependencies/filter.go to check ALL values before deciding for NotEqualTo comparator - Applied fix to both JSONTypeNumber and JSONTypeString cases - Added 4 comprehensive test cases proving the fix works correctly - All existing tests continue to pass with no regressions The fix implements true NOT IN semantics: - PASS: if value does not match ANY item in the array - FAIL: if value matches any item in the array Fixes #3808 Signed-off-by: James Lee <[email protected]>
1 parent 8ade11e commit 9590fe5

File tree

2 files changed

+164
-17
lines changed

2 files changed

+164
-17
lines changed

‎pkg/sensors/dependencies/filter.go‎

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,44 @@ filterData:
304304
}
305305

306306
case v1alpha1.JSONTypeNumber:
307+
// Special handling for NotEqualTo: check ALL values first
308+
if f.Comparator == v1alpha1.NotEqualTo {
309+
matchFound := false
310+
for _, value := range f.Value {
311+
filterVal, err := strconv.ParseFloat(value, 64)
312+
eventVal := pathResult.Float()
313+
if err != nil {
314+
if operator == v1alpha1.OrLogicalOperator {
315+
errMessages = append(errMessages, err.Error())
316+
continue filterData
317+
} else {
318+
return false, fmt.Errorf(errMsgTemplate, "data", err.Error())
319+
}
320+
}
321+
322+
if eventVal == filterVal {
323+
matchFound = true
324+
break
325+
}
326+
}
327+
328+
// For NotEqualTo: pass only if NO match was found (NOT IN semantics)
329+
if !matchFound {
330+
if operator == v1alpha1.OrLogicalOperator {
331+
return true, nil
332+
} else {
333+
continue filterData
334+
}
335+
}
336+
337+
if operator == v1alpha1.OrLogicalOperator {
338+
continue filterData
339+
} else {
340+
return false, nil
341+
}
342+
}
343+
344+
// Standard handling for all other comparators
307345
for _, value := range f.Value {
308346
filterVal, err := strconv.ParseFloat(value, 64)
309347
eventVal := pathResult.Float()
@@ -334,10 +372,6 @@ filterData:
334372
if eventVal <= filterVal {
335373
compareResult = true
336374
}
337-
case v1alpha1.NotEqualTo:
338-
if eventVal != filterVal {
339-
compareResult = true
340-
}
341375
case v1alpha1.EqualTo, v1alpha1.EmptyComparator:
342376
if eventVal == filterVal {
343377
compareResult = true
@@ -359,6 +393,43 @@ filterData:
359393
}
360394

361395
case v1alpha1.JSONTypeString:
396+
// Special handling for NotEqualTo: check ALL values first
397+
if f.Comparator == v1alpha1.NotEqualTo {
398+
matchFound := false
399+
for _, value := range f.Value {
400+
exp, err := regexp.Compile(value)
401+
if err != nil {
402+
if operator == v1alpha1.OrLogicalOperator {
403+
errMessages = append(errMessages, err.Error())
404+
continue filterData
405+
} else {
406+
return false, fmt.Errorf(errMsgTemplate, "data", err.Error())
407+
}
408+
}
409+
410+
if exp.Match([]byte(pathResult.String())) {
411+
matchFound = true
412+
break
413+
}
414+
}
415+
416+
// For NotEqualTo: pass only if NO match was found (NOT IN semantics)
417+
if !matchFound {
418+
if operator == v1alpha1.OrLogicalOperator {
419+
return true, nil
420+
} else {
421+
continue filterData
422+
}
423+
}
424+
425+
if operator == v1alpha1.OrLogicalOperator {
426+
continue filterData
427+
} else {
428+
return false, nil
429+
}
430+
}
431+
432+
// Standard handling for EqualTo comparator
362433
for _, value := range f.Value {
363434
exp, err := regexp.Compile(value)
364435
if err != nil {
@@ -370,20 +441,8 @@ filterData:
370441
}
371442
}
372443

373-
matchResult := false
374444
match := exp.Match([]byte(pathResult.String()))
375-
switch f.Comparator {
376-
case v1alpha1.EqualTo, v1alpha1.EmptyComparator:
377-
if match {
378-
matchResult = true
379-
}
380-
case v1alpha1.NotEqualTo:
381-
if !match {
382-
matchResult = true
383-
}
384-
}
385-
386-
if matchResult {
445+
if match {
387446
if operator == v1alpha1.OrLogicalOperator {
388447
return true, nil
389448
} else {

‎pkg/sensors/dependencies/filter_data_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,94 @@ func TestFilterData(t *testing.T) {
678678
expectedResult: true,
679679
expectErr: false,
680680
},
681+
{
682+
name: "string filter NotEqualTo with multiple values - value IN array (should FAIL with fix)",
683+
args: args{
684+
data: []v1alpha1.DataFilter{
685+
{
686+
Path: "k",
687+
Type: v1alpha1.JSONTypeString,
688+
Value: []string{"id1", "id2", "id3"},
689+
Comparator: "!=",
690+
},
691+
},
692+
operator: v1alpha1.EmptyLogicalOperator,
693+
event: &v1alpha1.Event{
694+
Context: &v1alpha1.EventContext{
695+
DataContentType: "application/json",
696+
},
697+
Data: []byte(`{"k": "id1"}`),
698+
},
699+
},
700+
expectedResult: false, // Should FAIL - value IS in array
701+
expectErr: false,
702+
},
703+
{
704+
name: "string filter NotEqualTo with multiple values - value NOT IN array (should PASS)",
705+
args: args{
706+
data: []v1alpha1.DataFilter{
707+
{
708+
Path: "k",
709+
Type: v1alpha1.JSONTypeString,
710+
Value: []string{"id1", "id2", "id3"},
711+
Comparator: "!=",
712+
},
713+
},
714+
operator: v1alpha1.EmptyLogicalOperator,
715+
event: &v1alpha1.Event{
716+
Context: &v1alpha1.EventContext{
717+
DataContentType: "application/json",
718+
},
719+
Data: []byte(`{"k": "id_other"}`),
720+
},
721+
},
722+
expectedResult: true, // Should PASS - value NOT in array
723+
expectErr: false,
724+
},
725+
{
726+
name: "number filter NotEqualTo with multiple values - value IN array (should FAIL with fix)",
727+
args: args{
728+
data: []v1alpha1.DataFilter{
729+
{
730+
Path: "k",
731+
Type: v1alpha1.JSONTypeNumber,
732+
Value: []string{"1.0", "2.0", "3.0"},
733+
Comparator: "!=",
734+
},
735+
},
736+
operator: v1alpha1.EmptyLogicalOperator,
737+
event: &v1alpha1.Event{
738+
Context: &v1alpha1.EventContext{
739+
DataContentType: "application/json",
740+
},
741+
Data: []byte(`{"k": 2.0}`),
742+
},
743+
},
744+
expectedResult: false, // Should FAIL - value IS in array
745+
expectErr: false,
746+
},
747+
{
748+
name: "number filter NotEqualTo with multiple values - value NOT IN array (should PASS)",
749+
args: args{
750+
data: []v1alpha1.DataFilter{
751+
{
752+
Path: "k",
753+
Type: v1alpha1.JSONTypeNumber,
754+
Value: []string{"1.0", "2.0", "3.0"},
755+
Comparator: "!=",
756+
},
757+
},
758+
operator: v1alpha1.EmptyLogicalOperator,
759+
event: &v1alpha1.Event{
760+
Context: &v1alpha1.EventContext{
761+
DataContentType: "application/json",
762+
},
763+
Data: []byte(`{"k": 5.0}`),
764+
},
765+
},
766+
expectedResult: true, // Should PASS - value NOT in array
767+
expectErr: false,
768+
},
681769
}
682770

683771
for _, test := range tests {

0 commit comments

Comments
 (0)