Allow empty expression evaluables as always-matching items (#10)

expr.go

@@ -131,6 +131,12 @@ func (a *aggregator) Evaluate(ctx context.Context, data map[string]any) ([]Evalu
 	// TODO: Concurrently match constant expressions using a semaphore for capacity.
 	for _, expr := range a.constants {
 		atomic.AddInt32(&matched, 1)
+
+		if expr.Evaluable.GetExpression() == "" {
+			result = append(result, expr.Evaluable)
+			continue
+		}
+
 		// NOTE: We don't need to add lifted expression variables,
 		// because match.Parsed.Evaluable() returns the original expression
 		// string.
@@ -249,6 +255,14 @@ func (a *aggregator) Add(ctx context.Context, eval Evaluable) (bool, error) {
 		return false, err
 	}
 
+	if eval.GetExpression() == "" {
+		// This is an empty expression which always matches.
+		a.lock.Lock()
+		a.constants = append(a.constants, parsed)
+		a.lock.Unlock()
+		return false, nil
+	}
+
 	aggregateable := true
 	for _, g := range parsed.RootGroups() {
 		ok, err := a.iterGroup(ctx, g, parsed, a.addNode)
@@ -273,6 +287,10 @@ func (a *aggregator) Add(ctx context.Context, eval Evaluable) (bool, error) {
 }
 
 func (a *aggregator) Remove(ctx context.Context, eval Evaluable) error {
+	if eval.GetExpression() == "" {
+		return a.removeConstantEvaluable(ctx, eval)
+	}
+
 	// parse the expression using our tree parser.
 	parsed, err := a.parser.Parse(ctx, eval)
 	if err != nil {
@@ -289,22 +307,9 @@ func (a *aggregator) Remove(ctx context.Context, eval Evaluable) error {
 			return err
 		}
 		if !ok && aggregateable {
-			// Find the index of the evaluable in constants and yank out.
-			idx := -1
-			for n, item := range a.constants {
-				if item.Evaluable.GetID() == eval.GetID() {
-					idx = n
-					break
-				}
-			}
-
-			if idx == -1 {
-				return ErrEvaluableNotFound
+			if err := a.removeConstantEvaluable(ctx, eval); err != nil {
+				return err
 			}
-
-			a.lock.Lock()
-			a.constants = append(a.constants[:idx], a.constants[idx+1:]...)
-			a.lock.Unlock()
 			aggregateable = false
 		}
 	}
@@ -316,6 +321,25 @@ func (a *aggregator) Remove(ctx context.Context, eval Evaluable) error {
 	return nil
 }
 
+func (a *aggregator) removeConstantEvaluable(ctx context.Context, eval Evaluable) error {
+	// Find the index of the evaluable in constants and yank out.
+	idx := -1
+	for n, item := range a.constants {
+		if item.Evaluable.GetID() == eval.GetID() {
+			idx = n
+			break
+		}
+	}
+	if idx == -1 {
+		return ErrEvaluableNotFound
+	}
+
+	a.lock.Lock()
+	a.constants = append(a.constants[:idx], a.constants[idx+1:]...)
+	a.lock.Unlock()
+	return nil
+}
+
 func (a *aggregator) iterGroup(ctx context.Context, node *Node, parsed *ParsedExpression, op nodeOp) (bool, error) {
 	if len(node.Ors) > 0 {
 		// If there are additional branches, don't bother to add this to the aggregate tree.

expr_test.go

@@ -20,13 +20,9 @@ func BenchmarkCachingEvaluate1_000(b *testing.B) {
 }
 
 // func BenchmarkNonCachingEvaluate1_000(b *testing.B) { benchEval(1_000, EnvParser(newEnv()), b) }
-
 func benchEval(i int, p CELParser, b *testing.B) {
 	for n := 0; n < b.N; n++ {
-		parser, err := NewTreeParser(p)
-		if err != nil {
-			panic(err)
-		}
+		parser := NewTreeParser(p)
 		_ = evaluate(b, i, parser)
 	}
 }
@@ -61,12 +57,11 @@ func evaluate(b *testing.B, i int, parser TreeParser) error {
 
 func TestEvaluate(t *testing.T) {
 	ctx := context.Background()
-	parser, err := NewTreeParser(NewCachingParser(newEnv(), nil))
-	require.NoError(t, err)
+	parser := NewTreeParser(NewCachingParser(newEnv(), nil))
 	e := NewAggregateEvaluator(parser, testBoolEvaluator)
 
 	expected := tex(`event.data.account_id == "yes" && event.data.match == "true"`)
-	_, err = e.Add(ctx, expected)
+	_, err := e.Add(ctx, expected)
 	require.NoError(t, err)
 
 	n := 100_000
@@ -116,12 +111,11 @@ func TestEvaluate(t *testing.T) {
 
 func TestEvaluate_Concurrently(t *testing.T) {
 	ctx := context.Background()
-	parser, err := NewTreeParser(NewCachingParser(newEnv(), nil))
-	require.NoError(t, err)
+	parser := NewTreeParser(NewCachingParser(newEnv(), nil))
 	e := NewAggregateEvaluator(parser, testBoolEvaluator)
 
 	expected := tex(`event.data.account_id == "yes" && event.data.match == "true"`)
-	_, err = e.Add(ctx, expected)
+	_, err := e.Add(ctx, expected)
 	require.NoError(t, err)
 
 	addOtherExpressions(100_000, e)
@@ -152,12 +146,11 @@ func TestEvaluate_Concurrently(t *testing.T) {
 
 func TestEvaluate_ArrayIndexes(t *testing.T) {
 	ctx := context.Background()
-	parser, err := NewTreeParser(NewCachingParser(newEnv(), nil))
-	require.NoError(t, err)
+	parser := NewTreeParser(NewCachingParser(newEnv(), nil))
 	e := NewAggregateEvaluator(parser, testBoolEvaluator)
 
 	expected := tex(`event.data.ids[1] == "id-b" && event.data.ids[2] == "id-c"`)
-	_, err = e.Add(ctx, expected)
+	_, err := e.Add(ctx, expected)
 	require.NoError(t, err)
 
 	t.Run("It doesn't return if arrays contain non-matching data", func(t *testing.T) {
@@ -199,8 +192,7 @@ func TestEvaluate_ArrayIndexes(t *testing.T) {
 
 func TestEvaluate_Compound(t *testing.T) {
 	ctx := context.Background()
-	parser, err := NewTreeParser(NewCachingParser(newEnv(), nil))
-	require.NoError(t, err)
+	parser := NewTreeParser(NewCachingParser(newEnv(), nil))
 	e := NewAggregateEvaluator(parser, testBoolEvaluator)
 
 	expected := tex(`event.data.a == "ok" && event.data.b == "yes" && event.data.c == "please"`)
@@ -449,6 +441,47 @@ func TestAddRemove(t *testing.T) {
 	})
 }
 
+func TestEmptyExpressions(t *testing.T) {
+	ctx := context.Background()
+	parser, err := newParser()
+	require.NoError(t, err)
+
+	e := NewAggregateEvaluator(parser, testBoolEvaluator)
+
+	empty := tex(``, "id-1")
+
+	t.Run("Adding an empty expression succeeds", func(t *testing.T) {
+		ok, err := e.Add(ctx, empty)
+		require.NoError(t, err)
+		require.False(t, ok)
+		require.Equal(t, 1, e.Len())
+		require.Equal(t, 1, e.ConstantLen())
+		require.Equal(t, 0, e.AggregateableLen())
+	})
+
+	t.Run("Empty expressions always match", func(t *testing.T) {
+		// Matching this expr should now fail.
+		eval, count, err := e.Evaluate(ctx, map[string]any{
+			"event": map[string]any{
+				"data": map[string]any{"any": true},
+			},
+		})
+		require.NoError(t, err)
+		require.EqualValues(t, 1, count)
+		require.EqualValues(t, 1, len(eval))
+		require.EqualValues(t, empty, eval[0])
+	})
+
+	t.Run("Removing an empty expression succeeds", func(t *testing.T) {
+		err := e.Remove(ctx, empty)
+		require.NoError(t, err)
+		require.Equal(t, 0, e.Len())
+		require.Equal(t, 0, e.ConstantLen())
+		require.Equal(t, 0, e.AggregateableLen())
+	})
+
+}
+
 // tex represents a test Evaluable expression
 func tex(expr string, ids ...string) Evaluable {
 	return testEvaluable{

parser.go

@@ -46,11 +46,11 @@ func (e envparser) Parse(txt string) (*cel.Ast, *cel.Issues, LiftedArgs) {
 }
 
 // NewTreeParser returns a new tree parser for a given *cel.Env
-func NewTreeParser(ep CELParser) (TreeParser, error) {
+func NewTreeParser(ep CELParser) TreeParser {
 	parser := &parser{
 		ep: ep,
 	}
-	return parser, nil
+	return parser
 }
 
 type parser struct {
@@ -63,7 +63,14 @@ type parser struct {
 }
 
 func (p *parser) Parse(ctx context.Context, eval Evaluable) (*ParsedExpression, error) {
-	ast, issues, vars := p.ep.Parse(eval.GetExpression())
+	expression := eval.GetExpression()
+	if expression == "" {
+		return &ParsedExpression{
+			Evaluable: eval,
+		}, nil
+	}
+
+	ast, issues, vars := p.ep.Parse(expression)
 	if issues != nil {
 		return nil, issues.Err()
 	}

parser_test.go

@@ -20,7 +20,7 @@ func newEnv() *cel.Env {
 }
 
 func newParser() (TreeParser, error) {
-	return NewTreeParser(EnvParser(newEnv()))
+	return NewTreeParser(EnvParser(newEnv())), nil
 }
 
 type parseTestInput struct {
@@ -1093,10 +1093,9 @@ func TestParse_LiftedVars(t *testing.T) {
 		t.Helper()
 
 		for _, test := range tests {
-			p, err := NewTreeParser(cachingCelParser)
+			p := NewTreeParser(cachingCelParser)
 			// overwrite rander so that the parser uses the same nil bytes
 			p.(*parser).rander = rander
-			require.NoError(t, err)
 			eval := tex(test.input)
 			actual, err := p.Parse(ctx, eval)