From 1bb7e92e16fbecd0f6479b2d1f99af7202f712d0 Mon Sep 17 00:00:00 2001
From: ryota-sakamoto <sakamo.ryota+github@gmail.com>
Date: Sun, 4 Apr 2021 02:58:31 +0900
Subject: [PATCH] feat: improve oncePer evaluate (#228)

Signed-off-by: Ryota Sakamoto <sakamo.ryota+github@gmail.com>
---
 docs/triggers.md             | 16 ++++++++++
 pkg/triggers/service.go      | 36 +++++++++++++---------
 pkg/triggers/service_test.go | 58 ++++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/docs/triggers.md b/docs/triggers.md
index c19b6c7d..3fdba606 100644
--- a/docs/triggers.md
+++ b/docs/triggers.md
@@ -64,6 +64,22 @@ data:
     send: [app-sync-succeeded]
 ```
 
+### oncePer
+
+The `oncePer` filed is supported like as follows.
+
+```yaml
+apiVersion: argoproj.io/v1alpha1
+kind: Application
+metadata:
+  annotations:
+    example.com/version: v0.1
+```
+
+```yaml
+oncePer: app.metadata.annotations["example.com/version"]
+```
+
 ## Default Triggers
 
 You can use `defaultTriggers` field instead of specifying individual triggers to the annotations.
diff --git a/pkg/triggers/service.go b/pkg/triggers/service.go
index 4ece68ba..d42e4cf3 100644
--- a/pkg/triggers/service.go
+++ b/pkg/triggers/service.go
@@ -4,9 +4,6 @@ import (
 	"crypto/sha1"
 	"encoding/base64"
 	"fmt"
-	"strings"
-
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"github.com/antonmedv/expr"
 	"github.com/antonmedv/expr/vm"
@@ -34,11 +31,16 @@ type Service interface {
 
 type service struct {
 	compiledConditions map[string]*vm.Program
+	compiledOncePer    map[string]*vm.Program
 	triggers           map[string][]Condition
 }
 
 func NewService(triggers map[string][]Condition) (*service, error) {
-	svc := service{map[string]*vm.Program{}, triggers}
+	svc := service{
+		compiledConditions: map[string]*vm.Program{},
+		compiledOncePer:    map[string]*vm.Program{},
+		triggers:           triggers,
+	}
 	for _, t := range triggers {
 		for _, condition := range t {
 			prog, err := expr.Compile(condition.When)
@@ -46,6 +48,14 @@ func NewService(triggers map[string][]Condition) (*service, error) {
 				return nil, err
 			}
 			svc.compiledConditions[condition.When] = prog
+
+			if condition.OncePer != "" {
+				prog, err := expr.Compile(condition.OncePer)
+				if err != nil {
+					return nil, err
+				}
+				svc.compiledOncePer[condition.OncePer] = prog
+			}
 		}
 	}
 	return &svc, nil
@@ -64,26 +74,24 @@ func (svc *service) Run(triggerName string, vars map[string]interface{}) ([]Cond
 	}
 	var res []ConditionResult
 	for i, condition := range t {
-		prog, ok := svc.compiledConditions[condition.When]
-		if !ok {
-			return nil, fmt.Errorf("trigger configiration has changed after initialization")
-		}
 		conditionResult := ConditionResult{
 			Templates: condition.Send,
 			Key:       fmt.Sprintf("[%d].%s", i, hash(condition.When)),
 		}
-		// ignore execution error and treat and false result
-		val, err := expr.Run(prog, vars)
-		if err == nil {
+
+		if prog, ok := svc.compiledConditions[condition.When]; !ok {
+			return nil, fmt.Errorf("trigger configiration has changed after initialization")
+		} else if val, err := expr.Run(prog, vars); err == nil { // ignore execution error and treat and false result
 			boolRes, ok := val.(bool)
 			conditionResult.Triggered = ok && boolRes
 		}
 
-		if condition.OncePer != "" {
-			if oncePer, ok, err := unstructured.NestedFieldNoCopy(vars, strings.Split(condition.OncePer, ".")...); err == nil && ok {
-				conditionResult.OncePer = fmt.Sprintf("%v", oncePer)
+		if prog, ok := svc.compiledOncePer[condition.OncePer]; ok {
+			if val, err := expr.Run(prog, vars); err == nil { // ignore execution error and treat and false result
+				conditionResult.OncePer = fmt.Sprintf("%v", val)
 			}
 		}
+
 		res = append(res, conditionResult)
 	}
 
diff --git a/pkg/triggers/service_test.go b/pkg/triggers/service_test.go
index 9d094a62..43dc3051 100644
--- a/pkg/triggers/service_test.go
+++ b/pkg/triggers/service_test.go
@@ -100,3 +100,61 @@ func TestRun_OncePerSet(t *testing.T) {
 		}}, res)
 	})
 }
+
+func TestRun_OncePer_Evaluate(t *testing.T) {
+	vars := map[string]interface{}{
+		"var1":     "abc",
+		"revision": "123",
+		"app": map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"annotations": map[string]interface{}{
+					"example.com/version": "v0.1",
+				},
+			},
+		},
+	}
+
+	tests := []struct {
+		OncePer string
+		Result  string
+	}{
+		{
+			OncePer: "revision",
+			Result:  "123",
+		},
+		{
+			OncePer: `app.metadata.annotations["example.com/version"]`,
+			Result:  "v0.1",
+		},
+	}
+
+	for _, tt := range tests {
+		svc, err := NewService(map[string][]Condition{
+			"my-trigger": {{
+				When:    "var1 == 'abc'",
+				Send:    []string{"my-template"},
+				OncePer: tt.OncePer,
+			}},
+		})
+
+		if !assert.NoError(t, err) {
+			t.FailNow()
+			return
+		}
+
+		conditionKey := fmt.Sprintf("[0].%s", hash("var1 == 'abc'"))
+
+		res, err := svc.Run("my-trigger", vars)
+		if !assert.NoError(t, err) {
+			t.FailNow()
+			return
+		}
+
+		assert.Equal(t, []ConditionResult{{
+			Key:       conditionKey,
+			Triggered: true,
+			Templates: []string{"my-template"},
+			OncePer:   tt.Result,
+		}}, res)
+	}
+}