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.

Sign up for GitHub

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

Jump to bottom

Adding Exclude property to DoubleRangeFilter and test coverage. #1268

Merged
merged 10 commits into from
Nov 2, 2020
Merged

Adding Exclude property to DoubleRangeFilter and test coverage. #1268

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cd1797a
Adding Exclude property to DoubleRangeFilter and test coverage.
adumovic Sep 11, 2020
8b5eda1
Merge branch 'master' into feature/ConfigurableExclusiveRanges
HazWard Oct 15, 2020
eb96ccd
Added comments to the exclude parameter
HazWard Oct 15, 2020
aea1517
Added tests for filters
HazWard Oct 20, 2020
eb5f78b
Fixed typo in comment
HazWard Oct 20, 2020
42031a1
Added more testcases
HazWard Oct 28, 2020
bde0ee8
Merge remote-tracking branch 'upstream/master' into feature/Configura…
HazWard Oct 28, 2020
cd1469f
Fixed wrong direction in comments for exclude bounds
HazWard Oct 28, 2020
ee228e2
Fixed conflicts
HazWard Nov 2, 2020
dc1f6d7
Merge remote-tracking branch 'upstream/master' into feature/Configura…
HazWard Nov 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions api/backend.swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,16 @@
],
"default": "UNKNOWN"
},
"DoubleRangeFilterExclude": {
"type": "string",
"enum": [
"NONE",
"MIN",
"MAX",
"BOTH"
],
"default": "NONE"
},
"openmatchAssignTicketsRequest": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -258,6 +268,10 @@
"type": "number",
"format": "double",
"description": "Minimum value."
},
"exclude": {
"$ref": "#/definitions/DoubleRangeFilterExclude",
"description": "Which bounds would be excluded when comparing with a ticket's search_fields.double_args value.\n\nBETA FEATURE WARNING: This field and the associated values are\nnot finalized and still subject to possible change or removal."
}
},
"title": "Filters numerical values to only those within a range.\n double_arg: \"foo\"\n max: 10\n min: 5\nmatches:\n {\"foo\": 5}\n {\"foo\": 7.5}\n {\"foo\": 10}\ndoes not match:\n {\"foo\": 4}\n {\"foo\": 10.01}\n {\"foo\": \"7.5\"}\n {}"
Expand Down
14 changes: 14 additions & 0 deletions api/matchfunction.swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@
}
},
"definitions": {
"DoubleRangeFilterExclude": {
"type": "string",
"enum": [
"NONE",
"MIN",
"MAX",
"BOTH"
],
"default": "NONE"
},
"openmatchAssignment": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -93,6 +103,10 @@
"type": "number",
"format": "double",
"description": "Minimum value."
},
"exclude": {
"$ref": "#/definitions/DoubleRangeFilterExclude",
"description": "Which bounds would be excluded when comparing with a ticket's search_fields.double_args value.\n\nBETA FEATURE WARNING: This field and the associated values are\nnot finalized and still subject to possible change or removal."
}
},
"title": "Filters numerical values to only those within a range.\n double_arg: \"foo\"\n max: 10\n min: 5\nmatches:\n {\"foo\": 5}\n {\"foo\": 7.5}\n {\"foo\": 10}\ndoes not match:\n {\"foo\": 4}\n {\"foo\": 10.01}\n {\"foo\": \"7.5\"}\n {}"
Expand Down
12 changes: 12 additions & 0 deletions api/messages.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ message DoubleRangeFilter {

// Minimum value.
double min = 3;

enum Exclude {
NONE = 0;
MIN = 1;
MAX = 2;
BOTH = 3;
}
// Which bounds would be excluded when comparing with a ticket's search_fields.double_args value.
//
// BETA FEATURE WARNING: This field and the associated values are
// not finalized and still subject to possible change or removal.
Exclude exclude = 4;
Laremere marked this conversation as resolved.
Show resolved Hide resolved
HazWard marked this conversation as resolved.
Show resolved Hide resolved
}

// Filters strings exactly equaling a value.
Expand Down
14 changes: 14 additions & 0 deletions api/query.swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@
}
},
"definitions": {
"DoubleRangeFilterExclude": {
"type": "string",
"enum": [
"NONE",
"MIN",
"MAX",
"BOTH"
],
"default": "NONE"
},
"openmatchAssignment": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -127,6 +137,10 @@
"type": "number",
"format": "double",
"description": "Minimum value."
},
"exclude": {
"$ref": "#/definitions/DoubleRangeFilterExclude",
"description": "Which bounds would be excluded when comparing with a ticket's search_fields.double_args value.\n\nBETA FEATURE WARNING: This field and the associated values are\nnot finalized and still subject to possible change or removal."
}
},
"title": "Filters numerical values to only those within a range.\n double_arg: \"foo\"\n max: 10\n min: 5\nmatches:\n {\"foo\": 5}\n {\"foo\": 7.5}\n {\"foo\": 10}\ndoes not match:\n {\"foo\": 4}\n {\"foo\": 10.01}\n {\"foo\": \"7.5\"}\n {}"
Expand Down
23 changes: 20 additions & 3 deletions internal/filter/filter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,27 @@ func (pf *PoolFilter) In(ticket *pb.Ticket) bool {
if !ok {
return false
}
// Not simplified so that NaN cases are handled correctly.
if !(v >= f.Min && v <= f.Max) {
return false

switch f.Exclude {
Laremere marked this conversation as resolved.
Show resolved Hide resolved
case pb.DoubleRangeFilter_NONE:
// Not simplified so that NaN cases are handled correctly.
if !(v >= f.Min && v <= f.Max) {
return false
}
case pb.DoubleRangeFilter_MIN:
if !(v > f.Min && v <= f.Max) {
return false
}
case pb.DoubleRangeFilter_MAX:
if !(v >= f.Min && v < f.Max) {
return false
}
case pb.DoubleRangeFilter_BOTH:
if !(v > f.Min && v < f.Max) {
return false
}
}

}

for _, f := range pf.StringEqualsFilters {
Expand Down
39 changes: 25 additions & 14 deletions internal/filter/testcases/testcases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,17 @@ func IncludedTestCases() []TestCase {
&pb.Pool{},
},

simpleDoubleRange("simpleInRange", 5, 0, 10),
simpleDoubleRange("exactMatch", 5, 5, 5),
simpleDoubleRange("infinityMax", math.Inf(1), 0, math.Inf(1)),
simpleDoubleRange("infinityMin", math.Inf(-1), math.Inf(-1), 0),
simpleDoubleRange("simpleInRange", 5, 0, 10, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("exactMatch", 5, 5, 5, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("infinityMax", math.Inf(1), 0, math.Inf(1), pb.DoubleRangeFilter_NONE),
simpleDoubleRange("infinityMin", math.Inf(-1), math.Inf(-1), 0, pb.DoubleRangeFilter_NONE),

simpleDoubleRange("inclusive both bounds lower", 0, 0, 1, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("inclusive both bounds upper", 1, 0, 1, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("exclusive min bounds upper", 1, 0, 1, pb.DoubleRangeFilter_MIN),
simpleDoubleRange("exclusive max bounds lower", 0, 0, 1, pb.DoubleRangeFilter_MAX),
simpleDoubleRange("exclusive both bounds upper", 2, 0, 3, pb.DoubleRangeFilter_BOTH),
simpleDoubleRange("exclusive both bounds lower", 1, 0, 3, pb.DoubleRangeFilter_BOTH),

{
"String equals simple positive",
Expand Down Expand Up @@ -182,7 +189,6 @@ func ExcludedTestCases() []TestCase {
},
},
},

{
"double range missing field",
&pb.Ticket{
Expand All @@ -202,15 +208,19 @@ func ExcludedTestCases() []TestCase {
},
},
},
simpleDoubleRange("valueTooLow", -1, 0, 10, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("valueTooHigh", 11, 0, 10, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("minIsNan", 5, math.NaN(), 10, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("maxIsNan", 5, 0, math.NaN(), pb.DoubleRangeFilter_NONE),
simpleDoubleRange("minMaxAreNan", 5, math.NaN(), math.NaN(), pb.DoubleRangeFilter_NONE),
simpleDoubleRange("valueIsNan", math.NaN(), 0, 10, pb.DoubleRangeFilter_NONE),
simpleDoubleRange("valueIsNanInfRange", math.NaN(), math.Inf(-1), math.Inf(1), pb.DoubleRangeFilter_NONE),
simpleDoubleRange("allAreNan", math.NaN(), math.NaN(), math.NaN(), pb.DoubleRangeFilter_NONE),

simpleDoubleRange("valueTooLow", -1, 0, 10),
simpleDoubleRange("valueTooHigh", 11, 0, 10),
simpleDoubleRange("minIsNan", 5, math.NaN(), 10),
simpleDoubleRange("maxIsNan", 5, 0, math.NaN()),
simpleDoubleRange("minMaxAreNan", 5, math.NaN(), math.NaN()),
simpleDoubleRange("valueIsNan", math.NaN(), 0, 10),
simpleDoubleRange("valueIsNanInfRange", math.NaN(), math.Inf(-1), math.Inf(1)),
simpleDoubleRange("allAreNan", math.NaN(), math.NaN(), math.NaN()),
simpleDoubleRange("exclusive upper bound", 1, 0, 1, pb.DoubleRangeFilter_MAX),
simpleDoubleRange("exclusive lower bound", 0, 0, 1, pb.DoubleRangeFilter_MIN),
simpleDoubleRange("exclusive both bounds lower", 0, 0, 1, pb.DoubleRangeFilter_BOTH),
simpleDoubleRange("exclusive both bounds upper", 1, 0, 1, pb.DoubleRangeFilter_BOTH),

{
"String equals simple negative", // and case sensitivity
Expand Down Expand Up @@ -329,7 +339,7 @@ func ExcludedTestCases() []TestCase {
}
}

func simpleDoubleRange(name string, value, min, max float64) TestCase {
func simpleDoubleRange(name string, value, min, max float64, exclude pb.DoubleRangeFilter_Exclude) TestCase {
return TestCase{
"double range " + name,
&pb.Ticket{
Expand All @@ -345,6 +355,7 @@ func simpleDoubleRange(name string, value, min, max float64) TestCase {
DoubleArg: "field",
Min: min,
Max: max,
Exclude: exclude,
},
},
},
Expand Down