Repository: apigee/registry
Pull request #588 (2022-06-01 07:11:48)

Extend CEL library with sum functions
Add support for `sum` functions in CEL
This is required because the conformance report stores errors and warning in lists.
While it is possible to derive errors for each guideLineReport in the form of lists, example:
```
[
   {
       guideLine: "sample guideline 1",
       errors: []errors,
   },
  {
       guideLine: "sample guideline 2",
       errors: []errors,
   }
]
```

We need an aggregate function which can sum all the error counts together to give us a final number.
See https://github.com/apigee/registry/issues/585 for more details

Future TODO:
Support `sum` for `double` lists.


Patch:
diff --git a/cmd/registry/scoring/expression.go b/cmd/registry/scoring/expression.go
index 6436085..d6336e5 100644
--- a/cmd/registry/scoring/expression.go
+++ b/cmd/registry/scoring/expression.go
@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	"github.com/apigee/registry/cmd/registry/core"
+	"github.com/apigee/registry/cmd/registry/scoring/extensions"
 	"github.com/apigee/registry/rpc"
 	"github.com/google/cel-go/cel"
 	metrics "github.com/google/gnostic/metrics"
@@ -28,7 +29,7 @@ import (
 
 // https://github.com/google/cel-spec/blob/master/doc/langdef.md#dynamic-values
 func evaluateScoreExpression(expression string, artifactMap map[string]interface{}) (interface{}, error) {
-	env, err := cel.NewEnv()
+	env, err := cel.NewEnv(extensions.Extensions())
 	if err != nil {
 		return nil, fmt.Errorf("error creating CEL environment: %s", err)
 	}
diff --git a/cmd/registry/scoring/extensions/extensions.go b/cmd/registry/scoring/extensions/extensions.go
new file mode 100644
index 0000000..c68a574
--- /dev/null
+++ b/cmd/registry/scoring/extensions/extensions.go
@@ -0,0 +1,49 @@
+package extensions
+
+import (
+	"github.com/google/cel-go/cel"
+	"github.com/google/cel-go/checker/decls"
+	"github.com/google/cel-go/interpreter/functions"
+	exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
+)
+
+func Extensions() cel.EnvOption {
+	return cel.Lib(extensionLib{})
+}
+
+type extensionLib struct{}
+
+func (extensionLib) CompileOptions() []cel.EnvOption {
+	return []cel.EnvOption{
+		cel.Declarations(
+			decls.NewFunction("sum",
+				decls.NewOverload("sum_int",
+					[]*exprpb.Type{decls.NewListType(decls.Int)},
+					decls.Int),
+			),
+		),
+	}
+}
+
+func (extensionLib) ProgramOptions() []cel.ProgramOption {
+	return []cel.ProgramOption{
+		cel.Functions(
+			&functions.Overload{
+				Operator: "sum",
+				Unary:    unary(function(sum_int, []string{"list<int>"}, "int")),
+			},
+			&functions.Overload{
+				Operator: "sum_int",
+				Unary:    unary(function(sum_int, []string{"list<int>"}, "int")),
+			},
+		),
+	}
+}
+
+func sum_int(vals []int64) (int64, error) {
+	var rv int64
+	for _, v := range vals {
+		rv = rv + v
+	}
+	return rv, nil
+}
diff --git a/cmd/registry/scoring/extensions/validators.go b/cmd/registry/scoring/extensions/validators.go
new file mode 100644
index 0000000..6681818
--- /dev/null
+++ b/cmd/registry/scoring/extensions/validators.go
@@ -0,0 +1,87 @@
+package extensions
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/google/cel-go/common/types"
+	"github.com/google/cel-go/common/types/ref"
+	"github.com/google/cel-go/interpreter/functions"
+)
+
+// holds the supported types
+var nativeTypeMap = map[string]reflect.Type{
+	"list<int>":    reflect.TypeOf([]int64{}),
+	"int":          reflect.TypeOf(int64(0)),
+	"list<double>": reflect.TypeOf([]float64{}),
+	"double":       reflect.TypeOf(float64(0)),
+}
+
+// Converts a go func into a CEL unary operation (single argument)
+func unary(fn functions.FunctionOp) functions.UnaryOp {
+	return func(arg ref.Val) ref.Val {
+		return fn(arg)
+	}
+}
+
+// Converts a go func into a CEL operation (any no. of arguments)
+func function(fn interface{}, paramTypes []string, returnType string) functions.FunctionOp {
+	return func(params ...ref.Val) ref.Val {
+		fnValue := reflect.ValueOf(fn)
+
+		// Check if the supplied, expected and implemented number of arguments are equal
+		if len(params) != len(paramTypes) || len(paramTypes) != fnValue.Type().NumIn() {
+			return types.NoSuchOverloadErr()
+		}
+
+		nativeParams := make([]reflect.Value, 0, len(params))
+
+		// Check the expected types for params
+		for i, p := range params {
+			expectedType, ok := nativeTypeMap[paramTypes[i]]
+			if !ok {
+				return types.NewErr("type %s is not supported in the extensions, please add support.", paramTypes[i])
+			}
+			// convert the supplied param into native type to validate
+			nativeP, err := p.ConvertToNative(expectedType)
+			if err != nil {
+				return types.NewErr(fmt.Sprintf("%s expects %dth argument to be of type %s", fn, i+1, expectedType))
+			}
+
+			// check if the native fn expects the supplied paramType
+			implType := fnValue.Type().In(i)
+			if implType != expectedType {
+				return types.NewErr(fmt.Sprintf("%dth argument of %s is configured to be of type %s, native fn expects %s", i+1, fn, expectedType, implType))
+			}
+
+			nativeParams = append(nativeParams, reflect.ValueOf(nativeP))
+		}
+
+		response := fnValue.Call(nativeParams)
+		if len(response) != 2 {
+			return types.NewErr("native function has %d return values, needs one plus error", len(response))
+		}
+
+		// Check for error
+		maybeErr := response[1].Interface()
+		err, ok := maybeErr.(error)
+		if maybeErr != nil && !ok {
+			return types.NewErr("second return value of native function must be error")
+		}
+		if err != nil {
+			return types.NewErr(err.Error())
+		}
+
+		// Check for returned value
+		expectedType, ok := nativeTypeMap[returnType]
+		if !ok {
+			return types.NewErr("type %s is not supported in the extensions, please add support.", returnType)
+		}
+		returnVal := response[0]
+		if returnVal.Type() != expectedType {
+			return types.NewErr("unexpected return type (got %s, expected %s)", returnVal.Type(), expectedType)
+		}
+
+		return types.DefaultTypeAdapter.NativeToValue(returnVal.Interface())
+	}
+}


Test patch:
diff --git a/cmd/registry/cmd/upload/artifact_test.go b/cmd/registry/cmd/upload/artifact_test.go
index 545a205..0654594 100644
--- a/cmd/registry/cmd/upload/artifact_test.go
+++ b/cmd/registry/cmd/upload/artifact_test.go
@@ -152,9 +152,9 @@ func TestScoreDefinitionArtifactUpload(t *testing.T) {
 				Formula: &rpc.ScoreDefinition_ScoreFormula{
 					ScoreFormula: &rpc.ScoreFormula{
 						Artifact: &rpc.ResourcePattern{
-							Pattern: "$resource.spec/artifacts/conformance-report",
+							Pattern: "$resource.spec/artifacts/conformance-apihub-styleguide",
 						},
-						ScoreExpression: "size(conformance.GuidelineReportGroups[2].RuleReportGroups[1])",
+						ScoreExpression: "sum(guidelineReportGroups[2].guidelineReports.map(r, has(r.ruleReportGroups[1].ruleReports) ? size(r.ruleReportGroups[1].ruleReports) : 0))",
 					},
 				},
 				Type: &rpc.ScoreDefinition_Integer{
diff --git a/cmd/registry/cmd/upload/testdata/score-definition.yaml b/cmd/registry/cmd/upload/testdata/score-definition.yaml
index 22d0958..0fe8928 100644
--- a/cmd/registry/cmd/upload/testdata/score-definition.yaml
+++ b/cmd/registry/cmd/upload/testdata/score-definition.yaml
@@ -18,8 +18,9 @@ target_resource:
   pattern: "apis/-/versions/-/specs/-"
 score_formula:
   artifact:
-    pattern: "$resource.spec/artifacts/conformance-report"
-  score_expression: "size(conformance.GuidelineReportGroups[2].RuleReportGroups[1])"
+    pattern: "$resource.spec/artifacts/conformance-apihub-styleguide"
+  # number of errors from conformance report
+  score_expression: "sum(guidelineReportGroups[2].guidelineReports.map(r, has(r.ruleReportGroups[1].ruleReports) ? size(r.ruleReportGroups[1].ruleReports) : 0))"
 integer: 
   min_value: 0
   max_value: 100
diff --git a/cmd/registry/scoring/extensions/extensions_test.go b/cmd/registry/scoring/extensions/extensions_test.go
new file mode 100644
index 0000000..0716752
--- /dev/null
+++ b/cmd/registry/scoring/extensions/extensions_test.go
@@ -0,0 +1,41 @@
+package extensions
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/google/cel-go/cel"
+)
+
+func TestSumInt(t *testing.T) {
+	expression := "sum([1,2,3])"
+
+	env, err := cel.NewEnv(Extensions())
+	if err != nil {
+		t.Fatalf("error creating CEL environment: %s", err)
+	}
+	ast, issues := env.Parse(expression)
+	if issues != nil && issues.Err() != nil {
+		t.Errorf("error parsing score_expression, %q: %s", expression, issues)
+		return
+	}
+	prg, err := env.Program(ast)
+	if err != nil {
+		t.Errorf("program construction error, %q: %s", expression, err)
+		return
+	}
+
+	out, _, err := prg.Eval(cel.NoVars())
+	if err != nil {
+		t.Errorf("evaluating expression %q returned unexpected error: %s", expression, err)
+		return
+	}
+	want := 6
+	nativeOut, err := out.ConvertToNative(reflect.TypeOf(want))
+	if err != nil {
+		t.Errorf("evaluating expression %q returned unexpected response type: want %s, got %s", expression, reflect.TypeOf(want), out.Type())
+	}
+	if nativeOut != want {
+		t.Errorf("evaluating expression %q returned unexpected response: want %d, got %d", expression, want, nativeOut)
+	}
+}
diff --git a/cmd/registry/scoring/extensions/validators_test.go b/cmd/registry/scoring/extensions/validators_test.go
new file mode 100644
index 0000000..3080efa
--- /dev/null
+++ b/cmd/registry/scoring/extensions/validators_test.go
@@ -0,0 +1,116 @@
+package extensions
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/google/cel-go/common/types"
+	"github.com/google/cel-go/common/types/ref"
+)
+
+func refVal(i interface{}) ref.Val {
+	return types.DefaultTypeAdapter.NativeToValue(i)
+}
+
+func TestFunction(t *testing.T) {
+	tests := []struct {
+		desc       string
+		fn         interface{}
+		paramTypes []string
+		returnType string
+		args       []ref.Val
+		wantErr    bool
+	}{
+		{
+			desc:       "fn(int, int) int",
+			fn:         func(x int64, y int64) (int64, error) { return x + y, nil },
+			paramTypes: []string{"int", "int"},
+			returnType: "int",
+			args:       []ref.Val{refVal(1), refVal(2)},
+		},
+		{
+			desc:       "fn(int, int) list<int>",
+			fn:         func(x int64, y int64) ([]int64, error) { return []int64{x, y}, nil },
+			paramTypes: []string{"int", "int"},
+			returnType: "list<int>",
+			args:       []ref.Val{refVal(1), refVal(2)},
+		},
+		{
+			desc:       "missing err in fn",
+			fn:         func(x int64, y int64) int64 { return x + y },
+			paramTypes: []string{"int", "int"},
+			returnType: "int",
+			args:       []ref.Val{refVal(1), refVal(2)},
+			wantErr:    true,
+		},
+		{
+			desc:       "non-error second arg in fn",
+			fn:         func(x int64, y int64) (int64, int64) { return x + y, 10 },
+			paramTypes: []string{"int", "int"},
+			returnType: "int",
+			args:       []ref.Val{refVal(1), refVal(2)},
+			wantErr:    true,
+		},
+		{
+			desc:       "paramType mismatch in implementation",
+			fn:         func(x int64, y int64) ([]int64, error) { return []int64{x, y}, nil },
+			paramTypes: []string{"double", "double"},
+			returnType: "list<int>",
+			args:       []ref.Val{refVal(0.1), refVal(0.2)},
+			wantErr:    true,
+		},
+		{
+			desc:       "returnType mismatch in implementation",
+			fn:         func(x int64, y int64) (int64, error) { return x + y, nil },
+			paramTypes: []string{"int", "int"},
+			returnType: "float64",
+			args:       []ref.Val{refVal(1), refVal(2)},
+			wantErr:    true,
+		},
+		{
+			desc:       "paramType mismatch in args",
+			fn:         func(x int64, y int64) ([]int64, error) { return []int64{x, y}, nil },
+			paramTypes: []string{"int", "int"},
+			returnType: "list<int>",
+			args:       []ref.Val{refVal(0.1), refVal(0.2)},
+			wantErr:    true,
+		},
+		{
+			desc:       "error from native func",
+			fn:         func(x int) (int, error) { return 0, fmt.Errorf("Sample error") },
+			paramTypes: []string{"int"},
+			returnType: "int",
+			args:       []ref.Val{refVal(1)},
+			wantErr:    true,
+		},
+		{
+			desc:       "unsupported paramType",
+			fn:         func(x int) (int, error) { return 0, nil },
+			paramTypes: []string{"int64"},
+			returnType: "int",
+			args:       []ref.Val{refVal(1)},
+			wantErr:    true,
+		},
+		{
+			desc:       "unsupported returnType",
+			fn:         func(x int) (int, error) { return 0, nil },
+			paramTypes: []string{"int"},
+			returnType: "int64",
+			args:       []ref.Val{refVal(1)},
+			wantErr:    true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			got := function(test.fn, test.paramTypes, test.returnType)(test.args...)
+			if test.wantErr && !types.IsError(got) {
+				t.Errorf("expected %s(%s) to return error", test.fn, test.args)
+			}
+
+			if !test.wantErr && types.IsError(got) {
+				t.Errorf("%s(%s) returned unexpected error: %s", test.fn, test.args, got.Value())
+			}
+		})
+	}
+}