From 32337124987de8b69743987518329349d4a5c85d Mon Sep 17 00:00:00 2001 From: Kilemonn Date: Wed, 18 Dec 2024 20:24:42 +0900 Subject: [PATCH 1/2] Add matches condition. Only compiling the regex once per Matches condition defined in the yaml. --- .../condition-action/condition_action.go | 18 ++++++++++++++---- .../matches_condition_action.go | 19 +++++++++++++++++++ condition/condition.go | 8 +++++++- condition/condition_test.go | 7 +++++-- condition/condition_type.go | 12 +++++++----- 5 files changed, 52 insertions(+), 12 deletions(-) create mode 100644 condition/condition-action/matches_condition_action.go diff --git a/condition/condition-action/condition_action.go b/condition/condition-action/condition_action.go index 7cc8370..09a39bd 100644 --- a/condition/condition-action/condition_action.go +++ b/condition/condition-action/condition_action.go @@ -1,11 +1,15 @@ package condition_action var ( - // TODO: Create a new instance for each different constraint definition that uses Unique - // Need to create and hold this variable, since it's state needs to be retained uniqueActionsMap map[string]UniqueConditionAction = make(map[string]UniqueConditionAction) + + matchesActionsMap map[string]MatchesConditionAction = make(map[string]MatchesConditionAction) ) +type ConditionAction interface { + CheckCondition(input string, args []string) bool +} + func GetUniqueInstance(id string) UniqueConditionAction { if entry, exists := uniqueActionsMap[id]; exists { return entry @@ -16,6 +20,12 @@ func GetUniqueInstance(id string) UniqueConditionAction { } } -type ConditionAction interface { - CheckCondition(input string, args []string) bool +func GetMatchesInstance(id string, pattern string) MatchesConditionAction { + if entry, exists := matchesActionsMap[id]; exists { + return entry + } else { + matchesAction := NewMatchesConditionAction(pattern) + matchesActionsMap[id] = matchesAction + return matchesAction + } } diff --git a/condition/condition-action/matches_condition_action.go b/condition/condition-action/matches_condition_action.go new file mode 100644 index 0000000..45aa25c --- /dev/null +++ b/condition/condition-action/matches_condition_action.go @@ -0,0 +1,19 @@ +package condition_action + +import "regexp" + +type MatchesConditionAction struct { + regex *regexp.Regexp +} + +func NewMatchesConditionAction(pattern string) MatchesConditionAction { + regex := regexp.MustCompile(pattern) + + return MatchesConditionAction{ + regex: regex, + } +} + +func (a MatchesConditionAction) CheckCondition(input string, args []string) bool { + return a.regex.Match([]byte(input)) +} diff --git a/condition/condition.go b/condition/condition.go index 221f0f3..f716871 100644 --- a/condition/condition.go +++ b/condition/condition.go @@ -64,5 +64,11 @@ func getArguments(arg string, expectedArgsCount uint) (args []string, err error) } func (c Condition) ApplyCondition(id string, input string) bool { - return c.Type.getConditionAction(id).CheckCondition(input, c.Args) + // TODO: Make this nicer, we are only passing in this arg for the matches condition so that we only need to compile + // the regex once on initialisation + arg := "" + if len(c.Args) > 0 { + arg = c.Args[0] + } + return c.Type.getConditionAction(id, arg).CheckCondition(input, c.Args) } diff --git a/condition/condition_test.go b/condition/condition_test.go index 21fda42..e618cc0 100644 --- a/condition/condition_test.go +++ b/condition/condition_test.go @@ -91,6 +91,9 @@ func TestApplyCondition(t *testing.T) { // Make sure an invalid constraint is forced to validate false to everything {"SomethingInvalid(arg1, arg2)", "invalid", true, []string{}, []string{"test test test", "1237532123", "true", "$!&@#($)"}}, + + {"Matches(\\d+)", "matches-regex", false, []string{"1234", "1", "testwith number 1"}, []string{"test", "no numbers++--"}}, + {"Matches([)", "invalid-regex", false, []string{"test"}, []string{"?"}}, } for _, c := range cases { @@ -102,10 +105,10 @@ func TestApplyCondition(t *testing.T) { } for _, successTest := range c.successInputs { - require.True(t, condition.ApplyCondition(c.id, successTest)) + require.True(t, condition.ApplyCondition(c.id, successTest), c.id+" - "+successTest) } for _, failTest := range c.failInputs { - require.False(t, condition.ApplyCondition(c.id, failTest)) + require.False(t, condition.ApplyCondition(c.id, failTest), c.id+" - "+failTest) } } } diff --git a/condition/condition_type.go b/condition/condition_type.go index 128d8e6..20ff835 100644 --- a/condition/condition_type.go +++ b/condition/condition_type.go @@ -16,6 +16,7 @@ const ( ConditionTypeHasSuffix ConditionType = iota ConditionTypeIsNumeric ConditionType = iota ConditionTypeIsBoolean ConditionType = iota + ConditionTypeMatches ConditionType = iota ) func ConditionTypeFromString(input string) ConditionType { @@ -28,7 +29,7 @@ func ConditionTypeFromString(input string) ConditionType { } func conditionTypeStrings() []string { - return []string{"invalid", "unique", "hasprefix", "hassuffix", "isnumeric", "isboolean"} + return []string{"invalid", "unique", "hasprefix", "hassuffix", "isnumeric", "isboolean", "matches"} } func (c ConditionType) IsValid() bool { @@ -36,11 +37,11 @@ func (c ConditionType) IsValid() bool { } func (c ConditionType) expectedArgsCount() uint { - args := []uint{0, 0, 1, 1, 0, 0} + args := []uint{0, 0, 1, 1, 0, 0, 1} return args[uint(c)] } -func (c ConditionType) conditionActions(id string) []condition_action.ConditionAction { +func (c ConditionType) conditionActions(id string, pattern string) []condition_action.ConditionAction { return []condition_action.ConditionAction{ condition_action.InvalidConditionAction{}, condition_action.GetUniqueInstance(id), @@ -48,9 +49,10 @@ func (c ConditionType) conditionActions(id string) []condition_action.ConditionA condition_action.HasSuffixConditionAction{}, condition_action.IsNumericConditionAction{}, condition_action.IsBooleanConditionAction{}, + condition_action.GetMatchesInstance(id, pattern), } } -func (c ConditionType) getConditionAction(id string) condition_action.ConditionAction { - return c.conditionActions(id)[uint(c)] +func (c ConditionType) getConditionAction(id string, pattern string) condition_action.ConditionAction { + return c.conditionActions(id, pattern)[uint(c)] } From 3ed01693a6c5e2e1b48eceaefd413026515b714f Mon Sep 17 00:00:00 2001 From: Kilemonn Date: Wed, 18 Dec 2024 20:28:23 +0900 Subject: [PATCH 2/2] Add log when matches cannot parse the regex and make sure it fails from then onward. Update test to confirm invalid regex behaviour. --- condition/condition-action/condition_action.go | 2 +- .../condition-action/matches_condition_action.go | 15 ++++++++++++--- condition/condition_test.go | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/condition/condition-action/condition_action.go b/condition/condition-action/condition_action.go index 09a39bd..24c0fab 100644 --- a/condition/condition-action/condition_action.go +++ b/condition/condition-action/condition_action.go @@ -24,7 +24,7 @@ func GetMatchesInstance(id string, pattern string) MatchesConditionAction { if entry, exists := matchesActionsMap[id]; exists { return entry } else { - matchesAction := NewMatchesConditionAction(pattern) + matchesAction := NewMatchesConditionAction(id, pattern) matchesActionsMap[id] = matchesAction return matchesAction } diff --git a/condition/condition-action/matches_condition_action.go b/condition/condition-action/matches_condition_action.go index 45aa25c..25235a8 100644 --- a/condition/condition-action/matches_condition_action.go +++ b/condition/condition-action/matches_condition_action.go @@ -1,13 +1,19 @@ package condition_action -import "regexp" +import ( + "fmt" + "regexp" +) type MatchesConditionAction struct { regex *regexp.Regexp } -func NewMatchesConditionAction(pattern string) MatchesConditionAction { - regex := regexp.MustCompile(pattern) +func NewMatchesConditionAction(id string, pattern string) MatchesConditionAction { + regex, err := regexp.Compile(pattern) + if err != nil { + fmt.Printf("Failed to initialise [Matches] condition's pattern with ID [%s], pattern [%s] with error [%s]. All condition checks will fail.", id, pattern, err.Error()) + } return MatchesConditionAction{ regex: regex, @@ -15,5 +21,8 @@ func NewMatchesConditionAction(pattern string) MatchesConditionAction { } func (a MatchesConditionAction) CheckCondition(input string, args []string) bool { + if a.regex == nil { + return false + } return a.regex.Match([]byte(input)) } diff --git a/condition/condition_test.go b/condition/condition_test.go index e618cc0..dfeace0 100644 --- a/condition/condition_test.go +++ b/condition/condition_test.go @@ -93,7 +93,7 @@ func TestApplyCondition(t *testing.T) { {"SomethingInvalid(arg1, arg2)", "invalid", true, []string{}, []string{"test test test", "1237532123", "true", "$!&@#($)"}}, {"Matches(\\d+)", "matches-regex", false, []string{"1234", "1", "testwith number 1"}, []string{"test", "no numbers++--"}}, - {"Matches([)", "invalid-regex", false, []string{"test"}, []string{"?"}}, + {"Matches([)", "invalid-regex", false, []string{}, []string{"everything should fail"}}, } for _, c := range cases {