Repository: apigee/registry
Pull request #563 (2022-05-03 00:42:40)

Add support for RollUpFormula

codecov[bot]: # [Codecov](https://codecov.io/gh/apigee/registry/pull/563?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) Report
> Merging [#563](https://codecov.io/gh/apigee/registry/pull/563?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (a0bcce7) into [main](https://codecov.io/gh/apigee/registry/commit/89946a113b5ed1124b40f26a61c39055ba13f5b1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (89946a1) will **increase** coverage by `0.16%`.
> The diff coverage is `74.07%`.

> :exclamation: Current head a0bcce7 differs from pull request most recent head a4371e2. Consider uploading reports for the commit a4371e2 to get more accurate results

```diff
@@            Coverage Diff             @@
##             main     #563      +/-   ##
==========================================
+ Coverage   41.42%   41.58%   +0.16%     
==========================================
  Files          72       72              
  Lines        8650     8688      +38     
==========================================
+ Hits         3583     3613      +30     
- Misses       4647     4651       +4     
- Partials      420      424       +4     
```


| [Impacted Files](https://codecov.io/gh/apigee/registry/pull/563?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) | Coverage Δ | |
|---|---|---|
| [cmd/registry/scoring/score.go](https://codecov.io/gh/apigee/registry/pull/563/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3Njb3Jpbmcvc2NvcmUuZ28=) | `55.79% <65.00%> (+1.51%)` | :arrow_up: |
| [cmd/registry/scoring/expression.go](https://codecov.io/gh/apigee/registry/pull/563/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3Njb3JpbmcvZXhwcmVzc2lvbi5nbw==) | `44.77% <100.00%> (-1.06%)` | :arrow_down: |
| [cmd/registry/scoring/parser.go](https://codecov.io/gh/apigee/registry/pull/563/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3Njb3JpbmcvcGFyc2VyLmdv) | `97.23% <100.00%> (+0.19%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apigee/registry/pull/563?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apigee/registry/pull/563?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee). Last update [89946a1...a4371e2](https://codecov.io/gh/apigee/registry/pull/563?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee).


Patch:
diff --git a/cmd/registry/scoring/expression.go b/cmd/registry/scoring/expression.go
index a63e0c4..6436085 100644
--- a/cmd/registry/scoring/expression.go
+++ b/cmd/registry/scoring/expression.go
@@ -26,13 +26,8 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
-func evaluateScoreExpression(expression, mimeType string, contents []byte) (interface{}, error) {
-	// https://github.com/google/cel-spec/blob/master/doc/langdef.md#dynamic-values
-	structProto, err := getMap(contents, mimeType)
-	if err != nil {
-		return nil, err
-	}
-
+// 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()
 	if err != nil {
 		return nil, fmt.Errorf("error creating CEL environment: %s", err)
@@ -47,7 +42,7 @@ func evaluateScoreExpression(expression, mimeType string, contents []byte) (inte
 		return nil, fmt.Errorf("program construction error, %q: %s", expression, err)
 	}
 
-	out, _, err := prg.Eval(structProto)
+	out, _, err := prg.Eval(artifactMap)
 	if err != nil {
 		return nil, fmt.Errorf("error in evaluating expression %q: %s", expression, err)
 	}
diff --git a/cmd/registry/scoring/parser.go b/cmd/registry/scoring/parser.go
index 0d22569..294c9a9 100644
--- a/cmd/registry/scoring/parser.go
+++ b/cmd/registry/scoring/parser.go
@@ -43,10 +43,16 @@ func ValidateScoreDefinition(ctx context.Context, parent string, scoreDefinition
 			errs := validateScoreFormula(targetName, formula.ScoreFormula)
 			totalErrs = append(totalErrs, errs...)
 		case *rpc.ScoreDefinition_RollupFormula:
+			if len(formula.RollupFormula.GetScoreFormulas()) == 0 {
+				totalErrs = append(totalErrs, fmt.Errorf("missing rollup_formula.score_formulas"))
+			}
 			for _, scoreFormula := range formula.RollupFormula.GetScoreFormulas() {
 				errs := validateScoreFormula(targetName, scoreFormula)
 				totalErrs = append(totalErrs, errs...)
 			}
+			if formula.RollupFormula.GetRollupExpression() == "" {
+				totalErrs = append(totalErrs, fmt.Errorf("missing rollup_formula.rollup_expression"))
+			}
 		default:
 			totalErrs = append(totalErrs, fmt.Errorf("missing formula, either 'score_formula' or 'rollup_formula' should be set"))
 		}
@@ -148,6 +154,14 @@ func validateScoreFormula(targetName patterns.ResourceName, scoreFormula *rpc.Sc
 		errs = append(errs, fmt.Errorf("invalid score_formula.artifact.pattern : %q, it should end with a resourceID and not a \"-\"", pattern))
 	}
 
+	if scoreFormula.GetScoreExpression() == "" {
+		errs = append(errs, fmt.Errorf("missing score_formula.score_expression"))
+	}
+
+	if refId := scoreFormula.GetReferenceId(); refId != "" && strings.Contains(refId, "-") {
+		errs = append(errs, fmt.Errorf("invalid score_formula.reference_id: %s, it should not contain hyphens '-'", refId))
+	}
+
 	return errs
 }
 
diff --git a/cmd/registry/scoring/score.go b/cmd/registry/scoring/score.go
index eb60de7..3305ae9 100644
--- a/cmd/registry/scoring/score.go
+++ b/cmd/registry/scoring/score.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"strconv"
+	"strings"
 
 	"github.com/apigee/registry/cmd/registry/core"
 	"github.com/apigee/registry/cmd/registry/patch"
@@ -159,12 +160,9 @@ func processFormula(
 	// Apply score formula
 	switch formula := definition.GetFormula().(type) {
 	case *rpc.ScoreDefinition_ScoreFormula:
-		scoreValue, err := processScoreFormula(ctx, client, formula.ScoreFormula, resource)
-		if err != nil {
-			return nil, err
-		}
-		return scoreValue, nil
-	// TODO: Add support for rollup_formula
+		return processScoreFormula(ctx, client, formula.ScoreFormula, resource)
+	case *rpc.ScoreDefinition_RollupFormula:
+		return processRollUpFormula(ctx, client, formula.RollupFormula, resource)
 	default:
 		return nil, fmt.Errorf("invalid formula in ScoreDefinition: {%v} ", formula)
 	}
@@ -175,16 +173,19 @@ func processScoreFormula(
 	client connection.Client,
 	formula *rpc.ScoreFormula,
 	resource patterns.ResourceInstance) (interface{}, error) {
-	// Fetch the artifact
 	extendedArtifact, err := patterns.SubstituteReferenceEntity(formula.GetArtifact().GetPattern(), resource.ResourceName())
 	if err != nil {
-		return nil, fmt.Errorf("invalid score_formula.artifact.pattern: %s, %s", formula.GetArtifact().GetPattern(), err)
+		return nil, fmt.Errorf("invalid score_formula.artifact.pattern: %s for {%v}, %s", formula.GetArtifact().GetPattern(), formula, err)
 	}
 	artifactName, err := names.ParseArtifact(extendedArtifact.String())
 	if err != nil {
-		return nil, fmt.Errorf("invalid score_formula.artifact.pattern: %s, %s", formula.GetArtifact().GetPattern(), err)
+		return nil, fmt.Errorf("invalid score_formula.artifact.pattern: %s for {%v}, %s", formula.GetArtifact().GetPattern(), formula, err)
+	}
+	if formula.GetScoreExpression() == "" {
+		return nil, fmt.Errorf("missing score_formula.score_expression for {%v}", formula)
 	}
 
+	// Fetch the artifact
 	var contents []byte
 	var mimeType string
 	err = core.GetArtifact(ctx, client, artifactName, true, func(artifact *rpc.Artifact) error {
@@ -198,13 +199,48 @@ func processScoreFormula(
 
 	// TODO: Add timestamp check. Compute the score only if the artifact has an update since the last score calculation
 
-	// Apply the score_expression
-	value, err := evaluateScoreExpression(formula.GetScoreExpression(), mimeType, contents)
+	// Convert artifact contents to map[string]interface{}
+	artifactMap, err := getMap(contents, mimeType)
 	if err != nil {
 		return nil, err
 	}
 
-	return value, nil
+	// Apply the score_expression
+	return evaluateScoreExpression(formula.GetScoreExpression(), artifactMap)
+}
+
+func processRollUpFormula(
+	ctx context.Context,
+	client connection.Client,
+	formula *rpc.RollUpFormula,
+	resource patterns.ResourceInstance) (interface{}, error) {
+	// Validate required fields
+	if len(formula.GetScoreFormulas()) == 0 {
+		return nil, fmt.Errorf("missing rollup_formula.score_formulas in {%v}", formula)
+	}
+	if formula.GetRollupExpression() == "" {
+		return nil, fmt.Errorf("missing rollup_formula.rollup_expression in {%v}", formula)
+	}
+
+	rollUpMap := make(map[string]interface{}, 0)
+	for _, f := range formula.GetScoreFormulas() {
+		scoreValue, err := processScoreFormula(ctx, client, f, resource)
+		if err != nil {
+			return nil, fmt.Errorf("error processing rollup_formula.score_formulas: %s", err)
+		}
+
+		refId := f.GetReferenceId()
+		if refId == "" {
+			return nil, fmt.Errorf("missing reference_id for score_formula {%v}", f)
+		}
+		if strings.Contains(refId, "-") {
+			return nil, fmt.Errorf("invalid reference_id for score_formula {%v}: cannot contain '-'", f)
+		}
+		rollUpMap[refId] = scoreValue
+	}
+
+	// Apply the rollup_expression
+	return evaluateScoreExpression(formula.GetRollupExpression(), rollUpMap)
 }
 
 func processScoreType(definition *rpc.ScoreDefinition, scoreValue interface{}, project string) (*rpc.Score, error) {


Test patch:
diff --git a/cmd/registry/scoring/expression_test.go b/cmd/registry/scoring/expression_test.go
index 52305de..1f246e2 100644
--- a/cmd/registry/scoring/expression_test.go
+++ b/cmd/registry/scoring/expression_test.go
@@ -121,24 +121,22 @@ func TestGetMapError(t *testing.T) {
 
 func TestEvaluateScoreExpression(t *testing.T) {
 	tests := []struct {
-		desc          string
-		expression    string
-		mimeType      string
-		contentsProto proto.Message
-		wantValue     interface{}
+		desc        string
+		expression  string
+		artifactMap map[string]interface{}
+		wantValue   interface{}
 	}{
 		{
 			desc:       "int happy path",
 			expression: "size(files[0].problems)",
-			mimeType:   "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.Lint",
-			contentsProto: &rpc.Lint{
-				Name: "openapi.yaml",
-				Files: []*rpc.LintFile{
+			artifactMap: map[string]interface{}{
+				"name": "openapi.yaml",
+				"files": []map[string]interface{}{
 					{
-						FilePath: "openapi.yaml",
-						Problems: []*rpc.LintProblem{
+						"filePath": "openapi.yaml",
+						"problems": []map[string]interface{}{
 							{
-								Message: "lint-error",
+								"message": "lint-error",
 							},
 						},
 					},
@@ -149,15 +147,14 @@ func TestEvaluateScoreExpression(t *testing.T) {
 		{
 			desc:       "double happy path",
 			expression: "double(size(files[0].problems))",
-			mimeType:   "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.Lint",
-			contentsProto: &rpc.Lint{
-				Name: "openapi.yaml",
-				Files: []*rpc.LintFile{
+			artifactMap: map[string]interface{}{
+				"name": "openapi.yaml",
+				"files": []map[string]interface{}{
 					{
-						FilePath: "openapi.yaml",
-						Problems: []*rpc.LintProblem{
+						"filePath": "openapi.yaml",
+						"problems": []map[string]interface{}{
 							{
-								Message: "lint-error",
+								"message": "lint-error",
 							},
 						},
 					},
@@ -168,15 +165,14 @@ func TestEvaluateScoreExpression(t *testing.T) {
 		{
 			desc:       "bool happy path",
 			expression: "size(files[0].problems)>0",
-			mimeType:   "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.Lint",
-			contentsProto: &rpc.Lint{
-				Name: "openapi.yaml",
-				Files: []*rpc.LintFile{
+			artifactMap: map[string]interface{}{
+				"name": "openapi.yaml",
+				"files": []map[string]interface{}{
 					{
-						FilePath: "openapi.yaml",
-						Problems: []*rpc.LintProblem{
+						"filePath": "openapi.yaml",
+						"problems": []map[string]interface{}{
 							{
-								Message: "lint-error",
+								"message": "lint-error",
 							},
 						},
 					},
@@ -188,9 +184,7 @@ func TestEvaluateScoreExpression(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			contents, _ := proto.Marshal(test.contentsProto)
-
-			gotValue, gotErr := evaluateScoreExpression(test.expression, test.mimeType, contents)
+			gotValue, gotErr := evaluateScoreExpression(test.expression, test.artifactMap)
 			if gotErr != nil {
 				t.Errorf("evaluateScoreExpression() returned unexpected error: %s", gotErr)
 			}
@@ -204,30 +198,21 @@ func TestEvaluateScoreExpression(t *testing.T) {
 
 func TestEvaluateScoreExpressionError(t *testing.T) {
 	tests := []struct {
-		desc          string
-		expression    string
-		mimeType      string
-		contentsProto proto.Message
+		desc        string
+		expression  string
+		artifactMap map[string]interface{}
 	}{
-		{
-			desc:       "error in getMap()",
-			expression: "",
-			// getMap() will return an error since ScoreDefinition is an unsupported type
-			mimeType:      "application/octet-stream;type=google.cloud.apigeeregistry.v1.scoring.ScoreDefinition",
-			contentsProto: &rpc.ScoreDefinition{},
-		},
 		{
 			desc:       "invalid field reference",
 			expression: "size(files.problems)", // correct expression should be "size(files[0].problems)"
-			mimeType:   "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.Lint",
-			contentsProto: &rpc.Lint{
-				Name: "openapi.yaml",
-				Files: []*rpc.LintFile{
+			artifactMap: map[string]interface{}{
+				"name": "openapi.yaml",
+				"files": []map[string]interface{}{
 					{
-						FilePath: "openapi.yaml",
-						Problems: []*rpc.LintProblem{
+						"filePath": "openapi.yaml",
+						"problems": []map[string]interface{}{
 							{
-								Message: "lint-error",
+								"message": "lint-error",
 							},
 						},
 					},
@@ -237,15 +222,14 @@ func TestEvaluateScoreExpressionError(t *testing.T) {
 		{
 			desc:       "unsupported type (list)",
 			expression: "files[0].problems", //this will return a list value
-			mimeType:   "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.Lint",
-			contentsProto: &rpc.Lint{
-				Name: "openapi.yaml",
-				Files: []*rpc.LintFile{
+			artifactMap: map[string]interface{}{
+				"name": "openapi.yaml",
+				"files": []map[string]interface{}{
 					{
-						FilePath: "openapi.yaml",
-						Problems: []*rpc.LintProblem{
+						"filePath": "openapi.yaml",
+						"problems": []map[string]interface{}{
 							{
-								Message: "lint-error",
+								"message": "lint-error",
 							},
 						},
 					},
@@ -256,11 +240,9 @@ func TestEvaluateScoreExpressionError(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			contents, _ := proto.Marshal(test.contentsProto)
-
-			_, gotErr := evaluateScoreExpression(test.expression, test.mimeType, contents)
+			_, gotErr := evaluateScoreExpression(test.expression, test.artifactMap)
 			if gotErr == nil {
-				t.Errorf("evaluateScoreExpression(%s, %s, %v) did not return an error", test.expression, test.mimeType, test.contentsProto)
+				t.Errorf("evaluateScoreExpression(%s, %v) did not return an error", test.expression, test.artifactMap)
 			}
 		})
 	}
diff --git a/cmd/registry/scoring/parser_test.go b/cmd/registry/scoring/parser_test.go
index 7b47949..1e8f3c9 100644
--- a/cmd/registry/scoring/parser_test.go
+++ b/cmd/registry/scoring/parser_test.go
@@ -552,7 +552,139 @@ func TestValidateScoreDefinition(t *testing.T) {
 			// This is expected here since it is not possible to validate ScoreFormula patterns if there are errors in the targetResource pattern.
 			wantNumErr: 2,
 		},
-		// Missing oneof components
+		// Missing components
+		{
+			desc:   "missing target resource",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/conformance-report",
+						},
+						ScoreExpression: "count(errors)",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+						Thresholds: []*rpc.NumberThreshold{
+							{
+								Severity: rpc.Severity_OK,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 0,
+									Max: 59,
+								},
+							},
+							{
+								Severity: rpc.Severity_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 60,
+									Max: 100,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:   "missing rollup_formula.rollup_expression",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				Formula: &rpc.ScoreDefinition_RollupFormula{
+					RollupFormula: &rpc.RollUpFormula{
+						ScoreFormulas: []*rpc.ScoreFormula{
+							{
+								Artifact: &rpc.ResourcePattern{
+									Pattern: "$resource.spec/artifacts/conformance-report",
+								},
+								ScoreExpression: "count(errors)",
+								ReferenceId:     "lint_errors",
+							},
+							{
+								Artifact: &rpc.ResourcePattern{
+									Pattern: "$resource.spec/artifacts/conformance-report",
+								},
+								ScoreExpression: "count(warnings)",
+								ReferenceId:     "lint_warnings",
+							},
+						},
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+						Thresholds: []*rpc.NumberThreshold{
+							{
+								Severity: rpc.Severity_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 61,
+									Max: 100,
+								},
+							},
+							{
+								Severity: rpc.Severity_WARNING,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 0,
+									Max: 60,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:   "missing rollup_formula.score_formulas",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				Formula: &rpc.ScoreDefinition_RollupFormula{
+					RollupFormula: &rpc.RollUpFormula{
+						RollupExpression: "lint_errors+lint_warnings",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+						Thresholds: []*rpc.NumberThreshold{
+							{
+								Severity: rpc.Severity_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 61,
+									Max: 100,
+								},
+							},
+							{
+								Severity: rpc.Severity_WARNING,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 0,
+									Max: 60,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
 		{
 			desc:   "missing formula",
 			parent: "projects/demo/locations/global",
@@ -712,6 +844,20 @@ func TestValidateScoreFormula(t *testing.T) {
 			},
 			wantNumErr: 1,
 		},
+		{
+			desc: "invalid reference_id",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/conformance-report",
+				},
+				ScoreExpression: "count(errors)",
+				ReferenceId:     "num-errors",
+			},
+			wantNumErr: 1,
+		},
 		// Combination errors
 		{
 			desc: "invalid pattern and missing name",
@@ -752,6 +898,29 @@ func TestValidateScoreFormula(t *testing.T) {
 			},
 			wantNumErr: 2,
 		},
+		// missing components
+		{
+			desc: "missing artifact",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc: "missing score expression",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/conformance-report",
+				},
+			},
+			wantNumErr: 1,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
diff --git a/cmd/registry/scoring/score_test.go b/cmd/registry/scoring/score_test.go
index 335ebe7..bba70db 100644
--- a/cmd/registry/scoring/score_test.go
+++ b/cmd/registry/scoring/score_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry/names"
 	"github.com/golang/protobuf/proto"
+	metrics "github.com/google/gnostic/metrics"
 	"github.com/google/go-cmp/cmp"
 	"google.golang.org/protobuf/testing/protocmp"
 )
@@ -210,7 +211,7 @@ func TestMatchResourceWithTarget(t *testing.T) {
 			resourceName, _ := patterns.ParseResourcePattern(test.resourceName)
 			gotErr := matchResourceWithTarget(test.targetPattern, resourceName, "projects/pattern-test/locations/global")
 			if test.wantErr && gotErr == nil {
-				t.Errorf("expected matchResourceWithTarget() to return errors")
+				t.Errorf("matchResourceWithTarget(%s, %v, %s) did not return an error", test.targetPattern, resourceName, "projects/pattern-test/locations/global")
 			}
 
 			if !test.wantErr && gotErr != nil {
@@ -224,14 +225,12 @@ func TestProcessScoreFormula(t *testing.T) {
 	ctx := context.Background()
 	registryClient, err := connection.NewClient(ctx)
 	if err != nil {
-		t.Logf("Failed to create client: %+v", err)
-		t.FailNow()
+		t.Fatalf("Failed to create client: %+v", err)
 	}
 	defer registryClient.Close()
 	adminClient, err := connection.NewAdminClient(ctx)
 	if err != nil {
-		t.Logf("Failed to create client: %+v", err)
-		t.FailNow()
+		t.Fatalf("Failed to create client: %+v", err)
 	}
 	defer adminClient.Close()
 
@@ -373,6 +372,37 @@ func TestProcessScoreFormulaError(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "unsupported artifact type",
+			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
+				deleteProject(ctx, adminClient, t, "score-formula-test")
+				createProject(ctx, adminClient, t, "score-formula-test")
+				createApi(ctx, client, t, "projects/score-formula-test/locations/global", "petstore")
+				createVersion(ctx, client, t, "projects/score-formula-test/locations/global/apis/petstore", "1.0.0")
+				createSpec(ctx, client, t, "projects/score-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+				artifactBytes, _ := proto.Marshal(&rpc.ScoreDefinition{
+					Id:             "dummy-score-definition",
+					TargetResource: &rpc.ResourcePattern{},
+					Formula:        nil,
+					Type:           nil,
+				})
+				createUpdateArtifact(
+					ctx, client, t,
+					"projects/score-formula-test/locations/global/apis/petstore/versions/1.0.0/specs//openapi.yaml/artifacts/score-definition",
+					artifactBytes, "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.ScoreDefinition")
+			},
+			formula: &rpc.ScoreFormula{},
+			resource: patterns.SpecResource{
+				SpecName: patterns.SpecName{
+					Name: names.Spec{
+						ProjectID: "score-formula-test",
+						ApiID:     "petstore",
+						VersionID: "1.0.0",
+						SpecID:    "openapi.yaml",
+					},
+				},
+			},
+		},
 		{
 			desc: "invalid expression",
 			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
@@ -416,6 +446,48 @@ func TestProcessScoreFormulaError(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "missing expression",
+			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
+				deleteProject(ctx, adminClient, t, "score-formula-test")
+				createProject(ctx, adminClient, t, "score-formula-test")
+				createApi(ctx, client, t, "projects/score-formula-test/locations/global", "petstore")
+				createVersion(ctx, client, t, "projects/score-formula-test/locations/global/apis/petstore", "1.0.0")
+				createSpec(ctx, client, t, "projects/score-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+				artifactBytes, _ := proto.Marshal(&rpc.Lint{
+					Name: "openapi.yaml",
+					Files: []*rpc.LintFile{
+						{
+							FilePath: "openapi.yaml",
+							Problems: []*rpc.LintProblem{
+								{
+									Message: "lint-error",
+								},
+							},
+						},
+					},
+				})
+				createUpdateArtifact(
+					ctx, client, t,
+					"projects/score-formula-test/locations/global/apis/petstore/versions/1.0.0/specs//openapi.yaml/artifacts/lint-spectral",
+					artifactBytes, "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.Lint")
+			},
+			formula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/lint-spectral",
+				},
+			},
+			resource: patterns.SpecResource{
+				SpecName: patterns.SpecName{
+					Name: names.Spec{
+						ProjectID: "score-formula-test",
+						ApiID:     "petstore",
+						VersionID: "1.0.0",
+						SpecID:    "openapi.yaml",
+					},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -423,14 +495,12 @@ func TestProcessScoreFormulaError(t *testing.T) {
 			ctx := context.Background()
 			registryClient, err := connection.NewClient(ctx)
 			if err != nil {
-				t.Logf("Failed to create client: %+v", err)
-				t.FailNow()
+				t.Fatalf("Failed to create client: %+v", err)
 			}
 			defer registryClient.Close()
 			adminClient, err := connection.NewAdminClient(ctx)
 			if err != nil {
-				t.Logf("Failed to create client: %+v", err)
-				t.FailNow()
+				t.Fatalf("Failed to create client: %+v", err)
 			}
 			defer adminClient.Close()
 
@@ -438,7 +508,312 @@ func TestProcessScoreFormulaError(t *testing.T) {
 
 			_, gotErr := processScoreFormula(ctx, registryClient, test.formula, test.resource)
 			if gotErr == nil {
-				t.Errorf("expected processScoreFormula() to return an error")
+				t.Errorf("processScoreFormula(ctx, client, %v, %v) did not return an error", test.formula, test.resource)
+			}
+		})
+	}
+}
+
+func TestProcessRollUpFormula(t *testing.T) {
+	ctx := context.Background()
+	registryClient, err := connection.NewClient(ctx)
+	if err != nil {
+		t.Fatalf("Failed to create client: %+v", err)
+	}
+	defer registryClient.Close()
+	adminClient, err := connection.NewAdminClient(ctx)
+	if err != nil {
+		t.Fatalf("Failed to create client: %+v", err)
+	}
+	defer adminClient.Close()
+
+	//setup
+	deleteProject(ctx, adminClient, t, "rollup-formula-test")
+	createProject(ctx, adminClient, t, "rollup-formula-test")
+	createApi(ctx, registryClient, t, "projects/rollup-formula-test/locations/global", "petstore")
+	createVersion(ctx, registryClient, t, "projects/rollup-formula-test/locations/global/apis/petstore", "1.0.0")
+	createSpec(ctx, registryClient, t, "projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+	// create lint artifact
+	lintBytes, _ := proto.Marshal(&rpc.Lint{
+		Name: "openapi.yaml",
+		Files: []*rpc.LintFile{
+			{
+				FilePath: "openapi.yaml",
+				Problems: []*rpc.LintProblem{
+					{
+						Message: "lint-error",
+					},
+					{
+						Message: "lint-error",
+					},
+				},
+			},
+		},
+	})
+	createUpdateArtifact(
+		ctx, registryClient, t,
+		"projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0/specs/openapi.yaml/artifacts/lint-spectral",
+		lintBytes, "application/octet-stream;type=google.cloud.apigeeregistry.applications.v1alpha1.Lint")
+	// create complexity artifact
+	complexityBytes, _ := proto.Marshal(&metrics.Complexity{
+		GetCount:    1,
+		PostCount:   1,
+		PutCount:    1,
+		DeleteCount: 1,
+	})
+	createUpdateArtifact(
+		ctx, registryClient, t,
+		"projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0/specs/openapi.yaml/artifacts/complexity",
+		complexityBytes, "application/octet-stream;type=gnostic.metrics.Complexity")
+
+	// arguments
+	formula := &rpc.RollUpFormula{
+		ScoreFormulas: []*rpc.ScoreFormula{
+			{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/lint-spectral",
+				},
+				ScoreExpression: "size(files[0].problems)",
+				ReferenceId:     "numErrors",
+			},
+			{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/complexity",
+				},
+				ScoreExpression: "getCount + postCount + putCount + deleteCount",
+				ReferenceId:     "numOperations",
+			},
+		},
+		RollupExpression: "double(numErrors)/numOperations",
+	}
+	resource := patterns.SpecResource{
+		SpecName: patterns.SpecName{
+			Name: names.Spec{
+				ProjectID: "rollup-formula-test",
+				ApiID:     "petstore",
+				VersionID: "1.0.0",
+				SpecID:    "openapi.yaml",
+			},
+		},
+	}
+	gotValue, gotErr := processRollUpFormula(ctx, registryClient, formula, resource)
+	if gotErr != nil {
+		t.Errorf("processRollUpFormula() returned unexpected error: %s", gotErr)
+	}
+
+	wantValue := float64(0.5)
+
+	if gotValue != nil && wantValue != gotValue {
+		t.Errorf("processRollUpFormula() returned unexpected value, want: %v, got: %v", wantValue, gotValue)
+	}
+}
+
+func TestProcessRollUpFormulaError(t *testing.T) {
+	tests := []struct {
+		desc     string
+		setup    func(context.Context, connection.Client, connection.AdminClient)
+		formula  *rpc.RollUpFormula
+		resource patterns.ResourceInstance
+	}{
+		{
+			desc: "missing score_formulas",
+			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
+				deleteProject(ctx, adminClient, t, "rollup-formula-test")
+				createProject(ctx, adminClient, t, "rollup-formula-test")
+				createApi(ctx, client, t, "projects/rollup-formula-test/locations/global", "petstore")
+				createVersion(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore", "1.0.0")
+				createSpec(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+			},
+			formula: &rpc.RollUpFormula{
+				RollupExpression: "double(numErrors)/numOperations",
+			},
+			resource: patterns.SpecResource{
+				SpecName: patterns.SpecName{
+					Name: names.Spec{
+						ProjectID: "rollup-formula-test",
+						ApiID:     "petstore",
+						VersionID: "1.0.0",
+						SpecID:    "openapi.yaml",
+					},
+				},
+			},
+		},
+		{
+			desc: "missing rollup_expression",
+			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
+				deleteProject(ctx, adminClient, t, "rollup-formula-test")
+				createProject(ctx, adminClient, t, "rollup-formula-test")
+				createApi(ctx, client, t, "projects/rollup-formula-test/locations/global", "petstore")
+				createVersion(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore", "1.0.0")
+				createSpec(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+			},
+			formula: &rpc.RollUpFormula{
+				ScoreFormulas: []*rpc.ScoreFormula{
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/lint-spectral",
+						},
+						ScoreExpression: "size(files[0].problems)",
+						ReferenceId:     "numErrors",
+					},
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/complexity",
+						},
+						ScoreExpression: "getCount + postCount + putCount + deleteCount",
+						ReferenceId:     "numOperations",
+					},
+				},
+			},
+			resource: patterns.SpecResource{
+				SpecName: patterns.SpecName{
+					Name: names.Spec{
+						ProjectID: "rollup-formula-test",
+						ApiID:     "petstore",
+						VersionID: "1.0.0",
+						SpecID:    "openapi.yaml",
+					},
+				},
+			},
+		},
+		{
+			desc: "invalid score_expression",
+			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
+				deleteProject(ctx, adminClient, t, "rollup-formula-test")
+				createProject(ctx, adminClient, t, "rollup-formula-test")
+				createApi(ctx, client, t, "projects/rollup-formula-test/locations/global", "petstore")
+				createVersion(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore", "1.0.0")
+				createSpec(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+			},
+			formula: &rpc.RollUpFormula{
+				ScoreFormulas: []*rpc.ScoreFormula{
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/lint-spectral",
+						},
+						ScoreExpression: "size(files.problems)", // invalid field reference
+						ReferenceId:     "numErrors",
+					},
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/complexity",
+						},
+						ScoreExpression: "getCount + postCount + putCount + deleteCount",
+						ReferenceId:     "numOperations",
+					},
+				},
+				RollupExpression: "double(numErrors)/numOperations",
+			},
+			resource: patterns.SpecResource{
+				SpecName: patterns.SpecName{
+					Name: names.Spec{
+						ProjectID: "rollup-formula-test",
+						ApiID:     "petstore",
+						VersionID: "1.0.0",
+						SpecID:    "openapi.yaml",
+					},
+				},
+			},
+		},
+		{
+			desc: "invalid rollup_expression",
+			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
+				deleteProject(ctx, adminClient, t, "rollup-formula-test")
+				createProject(ctx, adminClient, t, "rollup-formula-test")
+				createApi(ctx, client, t, "projects/rollup-formula-test/locations/global", "petstore")
+				createVersion(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore", "1.0.0")
+				createSpec(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+			},
+			formula: &rpc.RollUpFormula{
+				ScoreFormulas: []*rpc.ScoreFormula{
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/lint-spectral",
+						},
+						ScoreExpression: "size(files[0].problems)",
+						ReferenceId:     "numErrors",
+					},
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/complexity",
+						},
+						ScoreExpression: "getCount + postCount + putCount + deleteCount",
+						ReferenceId:     "numOperations",
+					},
+				},
+				RollupExpression: "numError/numOperation", // should be numErrors/numOperations
+			},
+			resource: patterns.SpecResource{
+				SpecName: patterns.SpecName{
+					Name: names.Spec{
+						ProjectID: "rollup-formula-test",
+						ApiID:     "petstore",
+						VersionID: "1.0.0",
+						SpecID:    "openapi.yaml",
+					},
+				},
+			},
+		},
+		{
+			desc: "invalid reference_id",
+			setup: func(ctx context.Context, client connection.Client, adminClient connection.AdminClient) {
+				deleteProject(ctx, adminClient, t, "rollup-formula-test")
+				createProject(ctx, adminClient, t, "rollup-formula-test")
+				createApi(ctx, client, t, "projects/rollup-formula-test/locations/global", "petstore")
+				createVersion(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore", "1.0.0")
+				createSpec(ctx, client, t, "projects/rollup-formula-test/locations/global/apis/petstore/versions/1.0.0", "openapi.yaml", gzipOpenAPIv3)
+			},
+			formula: &rpc.RollUpFormula{
+				ScoreFormulas: []*rpc.ScoreFormula{
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/lint-spectral",
+						},
+						ScoreExpression: "size(files[0].problems)",
+						ReferenceId:     "num-errors",
+					},
+					{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/complexity",
+						},
+						ScoreExpression: "getCount + postCount + putCount + deleteCount",
+						ReferenceId:     "num-operations",
+					},
+				},
+				RollupExpression: "num-errors/num-operations",
+			},
+			resource: patterns.SpecResource{
+				SpecName: patterns.SpecName{
+					Name: names.Spec{
+						ProjectID: "rollup-formula-test",
+						ApiID:     "petstore",
+						VersionID: "1.0.0",
+						SpecID:    "openapi.yaml",
+					},
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			ctx := context.Background()
+			registryClient, err := connection.NewClient(ctx)
+			if err != nil {
+				t.Fatalf("Failed to create client: %+v", err)
+			}
+			defer registryClient.Close()
+			adminClient, err := connection.NewAdminClient(ctx)
+			if err != nil {
+				t.Fatalf("Failed to create client: %+v", err)
+			}
+			defer adminClient.Close()
+
+			test.setup(ctx, registryClient, adminClient)
+
+			_, gotErr := processRollUpFormula(ctx, registryClient, test.formula, test.resource)
+			if gotErr == nil {
+				t.Errorf("processRollUpFormula(ctx, client, %v, %v) did not return an error", test.formula, test.resource)
 			}
 		})
 	}
@@ -618,7 +993,7 @@ func TestProcessScoreTypeError(t *testing.T) {
 		t.Run(test.desc, func(t *testing.T) {
 			_, gotErr := processScoreType(test.definition, test.scoreValue, "projects/score-type-test/locations/global")
 			if gotErr == nil {
-				t.Errorf("expected processScoreType() to return an error")
+				t.Errorf("processScoreType(%v, %v, %s) did not return an error", test.definition, test.scoreValue, "projects/score-type-test/locations/global")
 			}
 		})
 	}