Repository: apigee/registry
Pull request #536 (2022-04-19 20:10:26)

Validation function for ScoreDefinition
Validation logic details are here:https://github.com/apigee/registry/issues/515#issuecomment-1101597784

Note: There are multiple commits in this PR due to merge conflicts while rebasing.  Most of the core changes are in the fourth commit [Add a validation function for ScoreDefinition](https://github.com/apigee/registry/pull/536/commits/3aed6c2221e4d767389a9d6d2e43c7679662e951)

codecov[bot]: # [Codecov](https://codecov.io/gh/apigee/registry/pull/536?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) Report
> Merging [#536](https://codecov.io/gh/apigee/registry/pull/536?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (d4d5027) into [main](https://codecov.io/gh/apigee/registry/commit/bcf637245b962b9e930e50093be19baf6938fbde?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (bcf6372) will **increase** coverage by `0.74%`.
> The diff coverage is `84.51%`.

```diff
@@            Coverage Diff             @@
##             main     #536      +/-   ##
==========================================
+ Coverage   40.39%   41.13%   +0.74%     
==========================================
  Files          68       69       +1     
  Lines        8023     8165     +142     
==========================================
+ Hits         3241     3359     +118     
- Misses       4381     4400      +19     
- Partials      401      406       +5     
```


| [Impacted Files](https://codecov.io/gh/apigee/registry/pull/536?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) | Coverage Δ | |
|---|---|---|
| [cmd/registry/patterns/resources.go](https://codecov.io/gh/apigee/registry/pull/536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3BhdHRlcm5zL3Jlc291cmNlcy5nbw==) | `26.57% <36.84%> (+2.10%)` | :arrow_up: |
| [cmd/registry/cmd/upload/artifact.go](https://codecov.io/gh/apigee/registry/pull/536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NtZC91cGxvYWQvYXJ0aWZhY3QuZ28=) | `37.56% <37.50%> (-0.66%)` | :arrow_down: |
| [cmd/registry/patterns/parser.go](https://codecov.io/gh/apigee/registry/pull/536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3BhdHRlcm5zL3BhcnNlci5nbw==) | `73.37% <53.84%> (-2.82%)` | :arrow_down: |
| [cmd/registry/controller/controller.go](https://codecov.io/gh/apigee/registry/pull/536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NvbnRyb2xsZXIvY29udHJvbGxlci5nbw==) | `76.95% <80.00%> (-1.31%)` | :arrow_down: |
| [cmd/registry/scoring/parser.go](https://codecov.io/gh/apigee/registry/pull/536/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L3Njb3JpbmcvcGFyc2VyLmdv) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apigee/registry/pull/536?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/536?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee). Last update [bcf6372...d4d5027](https://codecov.io/gh/apigee/registry/pull/536?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).

shrutiparabgoogle: Thanks you for the suggestion @seaneganx. I refactored with mostly the same structure as suggested, except for a few renames. Also, added a testcase for the case you caught in one of the comments.

Patch:
diff --git a/cmd/registry/cmd/upload/artifact.go b/cmd/registry/cmd/upload/artifact.go
index 5e1cc59..a874619 100644
--- a/cmd/registry/cmd/upload/artifact.go
+++ b/cmd/registry/cmd/upload/artifact.go
@@ -22,6 +22,7 @@ import (
 	"github.com/apigee/registry/cmd/registry/controller"
 	"github.com/apigee/registry/cmd/registry/core"
 	"github.com/apigee/registry/cmd/registry/patch"
+	"github.com/apigee/registry/cmd/registry/scoring"
 	"github.com/apigee/registry/connection"
 	"github.com/apigee/registry/log"
 	"github.com/apigee/registry/rpc"
@@ -97,7 +98,7 @@ func buildArtifact(ctx context.Context, parent string, filename string) (*rpc.Ar
 	case "ScoreCardDefinition", patch.ScoreCardDefinitionMimeType:
 		artifact, err = buildScoreCardDefinitionArtifact(ctx, jsonBytes)
 	case "ScoreDefinition", patch.ScoreDefinitionMimeType:
-		artifact, err = buildScoreDefinitionArtifact(ctx, jsonBytes)
+		artifact, err = buildScoreDefinitionArtifact(ctx, parent, jsonBytes)
 	case "TaxonomyList", patch.TaxonomyListMimeType:
 		artifact, err = buildTaxonomyListArtifact(ctx, jsonBytes)
 	default:
@@ -194,11 +195,19 @@ func buildTaxonomyListArtifact(ctx context.Context, jsonBytes []byte) (*rpc.Arti
 	}, nil
 }
 
-func buildScoreDefinitionArtifact(ctx context.Context, jsonBytes []byte) (*rpc.Artifact, error) {
+func buildScoreDefinitionArtifact(ctx context.Context, parent string, jsonBytes []byte) (*rpc.Artifact, error) {
 	m := &rpc.ScoreDefinition{}
 	if err := protojson.Unmarshal(jsonBytes, m); err != nil {
 		return nil, err
 	}
+	errs := scoring.ValidateScoreDefinition(ctx, parent, m)
+	if count := len(errs); count > 0 {
+		for _, err := range errs {
+			log.FromContext(ctx).WithError(err).Error("ScoreDefinition error")
+		}
+		return nil, fmt.Errorf("ScoreDefinition contains %d error(s): see logs for details", count)
+	}
+
 	artifactBytes, err := proto.Marshal(m)
 	if err != nil {
 		return nil, err
diff --git a/cmd/registry/controller/controller.go b/cmd/registry/controller/controller.go
index 4bc7dec..8750020 100644
--- a/cmd/registry/controller/controller.go
+++ b/cmd/registry/controller/controller.go
@@ -76,7 +76,7 @@ func processManifestResource(
 	resourcePattern := fmt.Sprintf("projects/%s/locations/global/%s", projectID, generatedResource.Pattern)
 	dependencyMaps := make([]map[string]time.Time, 0, len(generatedResource.Dependencies))
 	for _, dependency := range generatedResource.Dependencies {
-		dMap, err := generateDependencyMap(ctx, client, resourcePattern, dependency, projectID)
+		dMap, err := generateDependencyMap(ctx, client, resourcePattern, dependency)
 		if err != nil {
 			return nil, fmt.Errorf("error while generating dependency map for %v: %s", dependency, err)
 		}
@@ -94,8 +94,7 @@ func generateDependencyMap(
 	ctx context.Context,
 	client connection.Client,
 	resourcePattern string,
-	dependency *rpc.Dependency,
-	projectID string) (map[string]time.Time, error) {
+	dependency *rpc.Dependency) (map[string]time.Time, error) {
 	// Creates a map of the resources to group them into corresponding buckets
 	// of match pattern which store the maxTimestamp
 	// An example entry will look like this:
@@ -114,13 +113,13 @@ func generateDependencyMap(
 	}
 
 	// Extend the dependency pattern if it contains $resource.api like pattern
-	extDependencyQuery, err := patterns.SubstituteReferenceEntity(dependency.Pattern, resourceName, projectID)
+	extDependencyName, err := patterns.SubstituteReferenceEntity(dependency.Pattern, resourceName)
 	if err != nil {
 		return nil, err
 	}
 
 	// Fetch resources using the extDependencyQuery
-	sourceList, err := listResources(ctx, client, extDependencyQuery, dependency.Filter)
+	sourceList, err := listResources(ctx, client, extDependencyName.String(), dependency.Filter)
 	if err != nil {
 		return nil, err
 	}
@@ -139,7 +138,7 @@ func generateDependencyMap(
 	}
 
 	if len(sourceMap) == 0 {
-		return nil, fmt.Errorf("no resources found for pattern: %s, filer: %s", extDependencyQuery, dependency.Filter)
+		return nil, fmt.Errorf("no resources found for pattern: %s, filer: %s", extDependencyName.String(), dependency.Filter)
 	}
 
 	return sourceMap, nil
diff --git a/cmd/registry/patterns/parser.go b/cmd/registry/patterns/parser.go
index 22f79f7..5e53927 100644
--- a/cmd/registry/patterns/parser.go
+++ b/cmd/registry/patterns/parser.go
@@ -61,7 +61,7 @@ func ParseResourcePattern(resourcePattern string) (ResourceName, error) {
 	return nil, fmt.Errorf("invalid resourcePattern: %s", resourcePattern)
 }
 
-func SubstituteReferenceEntity(resourcePattern string, referred ResourceName, projectID string) (string, error) {
+func SubstituteReferenceEntity(resourcePattern string, referred ResourceName) (ResourceName, error) {
 	// Extends the resource pattern by replacing references to $resource
 	// Example:
 	// referred: "projects/demo/locations/global/apis/-/versions/-/specs/-/artifacts/-"
@@ -74,21 +74,29 @@ func SubstituteReferenceEntity(resourcePattern string, referred ResourceName, pr
 
 	entity, entityType, err := GetReferenceEntityType(resourcePattern)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	// no $resource reference present
 	// simply prepend the projectname and return full resource name
 	if entityType == "default" {
-		return fmt.Sprintf("projects/%s/locations/global/%s", projectID, resourcePattern), nil
+		resourceName, err := ParseResourcePattern(fmt.Sprintf("%s/locations/global/%s", referred.Project(), resourcePattern))
+		if err != nil {
+			return nil, err
+		}
+		return resourceName, nil
 	}
 
 	entityVal, err := GetReferenceEntityValue(resourcePattern, referred)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
-	return strings.Replace(resourcePattern, entity, entityVal, 1), nil
+	extendedName, err := ParseResourcePattern(strings.Replace(resourcePattern, entity, entityVal, 1))
+	if err != nil {
+		return nil, err
+	}
+	return extendedName, nil
 
 }
 
diff --git a/cmd/registry/patterns/resources.go b/cmd/registry/patterns/resources.go
index fa063a3..ae11111 100644
--- a/cmd/registry/patterns/resources.go
+++ b/cmd/registry/patterns/resources.go
@@ -29,6 +29,7 @@ type ResourceName interface {
 	Spec() string
 	Version() string
 	Api() string
+	Project() string
 	String() string
 	ParentName() ResourceName
 }
@@ -53,6 +54,10 @@ func (s SpecName) Api() string {
 	return s.Name.Api().String()
 }
 
+func (s SpecName) Project() string {
+	return s.Name.Project().String()
+}
+
 func (s SpecName) String() string {
 	return s.Name.String()
 }
@@ -91,6 +96,10 @@ func (v VersionName) Api() string {
 	return v.Name.Api().String()
 }
 
+func (v VersionName) Project() string {
+	return v.Name.Project().String()
+}
+
 func (v VersionName) String() string {
 	return v.Name.String()
 }
@@ -130,6 +139,10 @@ func (a ApiName) Api() string {
 	return a.Name.String()
 }
 
+func (a ApiName) Project() string {
+	return a.Name.Project().String()
+}
+
 func (a ApiName) String() string {
 	return a.Name.String()
 }
@@ -173,6 +186,10 @@ func (p ProjectName) Api() string {
 	return ""
 }
 
+func (p ProjectName) Project() string {
+	return p.Name.String()
+}
+
 func (p ProjectName) String() string {
 	return p.Name.String()
 }
@@ -238,6 +255,20 @@ func (ar ArtifactName) Api() string {
 	return ""
 }
 
+func (ar ArtifactName) Project() string {
+	projectPattern := names.Project{
+		ProjectID: ar.Name.ProjectID(),
+	}
+	// Validate the generated name
+	if _, err := names.ParseProject(projectPattern.String()); err == nil {
+		return projectPattern.String()
+	} else if _, err := names.ParseProjectCollection(projectPattern.String()); err == nil {
+		return projectPattern.String()
+	}
+
+	return ""
+}
+
 func (ar ArtifactName) String() string {
 	return ar.Name.String()
 }
diff --git a/cmd/registry/scoring/parser.go b/cmd/registry/scoring/parser.go
new file mode 100644
index 0000000..0c14533
--- /dev/null
+++ b/cmd/registry/scoring/parser.go
@@ -0,0 +1,189 @@
+package scoring
+
+import (
+	"context"
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/apigee/registry/rpc"
+
+	"github.com/apigee/registry/cmd/registry/patterns"
+)
+
+func ValidateScoreDefinition(ctx context.Context, parent string, scoreDefinition *rpc.ScoreDefinition) []error {
+	totalErrs := make([]error, 0)
+
+	// target_resource.pattern should be a valid resource pattern
+	targetName, err := patterns.ParseResourcePattern(fmt.Sprintf("%s/%s", parent, scoreDefinition.GetTargetResource().GetPattern()))
+	if err != nil {
+		totalErrs = append(totalErrs, err)
+	}
+
+	// TODO: Check for valid filter in target_resource
+
+	// Validate formula if there were no errors in target_resource.pattern
+	if len(totalErrs) == 0 {
+		switch formula := scoreDefinition.GetFormula().(type) {
+		case *rpc.ScoreDefinition_ScoreFormula:
+			errs := validateScoreFormula(targetName, formula.ScoreFormula)
+			totalErrs = append(totalErrs, errs...)
+		case *rpc.ScoreDefinition_RollupFormula:
+			for _, scoreFormula := range formula.RollupFormula.GetScoreFormulas() {
+				errs := validateScoreFormula(targetName, scoreFormula)
+				totalErrs = append(totalErrs, errs...)
+			}
+		default:
+			totalErrs = append(totalErrs, fmt.Errorf("missing formula, either 'score_formula' or 'rollup_formula' should be set"))
+		}
+	}
+
+	// Validate threshold
+	switch scoreType := scoreDefinition.GetType().(type) {
+	case *rpc.ScoreDefinition_Percent:
+		// minValue: 0 maxValue:100
+		// validate that the set thresholds are within these bounds
+		errs := validateNumberThresholds(scoreType.Percent.GetThresholds(), 0, 100)
+		totalErrs = append(totalErrs, errs...)
+	case *rpc.ScoreDefinition_Integer:
+		// defaults if not set: minValue: 0 maxValue:0
+		minValue := scoreType.Integer.GetMinValue()
+		maxValue := scoreType.Integer.GetMaxValue()
+		// if minValue==maxValue, means the score can take only one value, in that case integer is not the correct type.
+		// other types will be supported in the future (enums) to cover this case.
+		if minValue >= maxValue {
+			totalErrs = append(totalErrs, fmt.Errorf("invalid min_value(%d) and max_value(%d), min_value shoud be less than max_value", minValue, maxValue))
+		} else { // validate that the set thresholds are within minValue and maxValue limits
+			errs := validateNumberThresholds(scoreType.Integer.GetThresholds(), minValue, maxValue)
+			totalErrs = append(totalErrs, errs...)
+		}
+	case *rpc.ScoreDefinition_Boolean:
+		errs := validateBooleanThresholds(scoreType.Boolean.GetThresholds())
+		totalErrs = append(totalErrs, errs...)
+	default:
+		totalErrs = append(totalErrs, fmt.Errorf("missing type, either of 'percent', 'integer' or 'boolean' should be set"))
+	}
+
+	return totalErrs
+}
+
+func validateScoreFormula(targetName patterns.ResourceName, scoreFormula *rpc.ScoreFormula) []error {
+	errs := make([]error, 0)
+
+	// Validation checks for score_formula.artifact.pattern
+	pattern := scoreFormula.GetArtifact().GetPattern()
+
+	// pattern should have a valid $resource pattern
+	_, entityType, err := patterns.GetReferenceEntityType(pattern)
+	if err != nil {
+		errs = append(errs, fmt.Errorf("invalid score_formula.artifact.pattern: %s", err))
+	} else if entityType == "default" {
+		// pattern should always start with a $resource reference
+		errs = append(errs, fmt.Errorf("invalid score_formula.artifact.pattern: %q, must always start with '$resource.(api|version|spec|artifact)'", pattern))
+	} else if _, err = patterns.GetReferenceEntityValue(pattern, targetName); err != nil {
+		// $resource should have valid entity reference wrt target_resource
+		errs = append(errs, fmt.Errorf("invalid $resource reference in score_formula.artifact.pattern: %s", err))
+	}
+
+	// score_formula.pattern.pattern should not end with a "-"
+	if strings.HasSuffix(scoreFormula.GetArtifact().GetPattern(), "/-") {
+		errs = append(errs, fmt.Errorf("invalid score_formula.artifact.pattern : %q, it should end with a resourceID and not a \"-\"", pattern))
+	}
+
+	return errs
+}
+
+func validateNumberThresholds(thresholds []*rpc.NumberThreshold, minValue, maxValue int32) []error {
+
+	if len(thresholds) == 0 {
+		// no error returned since thresholds are optional
+		return []error{}
+	}
+
+	errs := make([]error, 0)
+
+	// keep track of the endpoints of the ranges
+	globalRangeMax := minValue
+	globalRangeMin := maxValue
+
+	for _, t := range thresholds {
+		// Check for validity of the range values
+		rangeMin := t.GetRange().GetMin()
+		rangeMax := t.GetRange().GetMax()
+
+		// min==max is valid here, one specific value can have a dedicated severity.
+		if rangeMin > rangeMax {
+			errs = append(errs, fmt.Errorf("invalid range [%d, %d]: range.min cannot be greater than range.max", rangeMin, rangeMax))
+			continue
+		}
+
+		// Check for out of limits conditions
+		if rangeMax > maxValue || rangeMax < minValue {
+			errs = append(errs, fmt.Errorf("invalid range [%d, %d]: range.max(%d) should be within min_value(%d) and max_value(%d) limits", rangeMin, rangeMax, rangeMax, minValue, maxValue))
+		}
+		if rangeMin < minValue || rangeMin > maxValue {
+			errs = append(errs, fmt.Errorf("invalid range [%d, %d]: range.min(%d) should be within min_value(%d) and max_value(%d) limits", rangeMin, rangeMax, rangeMin, minValue, maxValue))
+		}
+
+		// Update global min and max
+		if rangeMin <= globalRangeMin {
+			globalRangeMin = rangeMin
+		}
+		if rangeMax >= globalRangeMax {
+			globalRangeMax = rangeMax
+		}
+	}
+
+	// Don't check for missing coverage or threshold overlaps unless all the ranges are valid and in bounds.
+	if len(errs) > 0 {
+		return errs
+	}
+
+	// sort the thresholds based on range.min
+	sort.Slice(thresholds, func(i, j int) bool {
+		return thresholds[i].GetRange().GetMin() < thresholds[j].GetRange().GetMin()
+	})
+
+	// Check for overlaps and gaps
+	for i := 0; i < len(thresholds)-1; i++ {
+		left, right := thresholds[i].GetRange(), thresholds[i+1].GetRange()
+		if left.GetMax() < right.GetMin()-1 {
+			errs = append(errs, fmt.Errorf("incomplete coverage: missing coverage between %d and %d", left.GetMax(), right.GetMin()))
+		} else if left.GetMax() > right.GetMin()-1 {
+			errs = append(errs, fmt.Errorf("invalid thresholds [%d, %d] and [%d, %d]: thresholds must not overlap", left.GetMin(), left.GetMax(), right.GetMin(), right.GetMax()))
+		}
+	}
+
+	// Finally check for gaps at the endpoints
+	if globalRangeMax < maxValue {
+		errs = append(errs, fmt.Errorf("incomplete coverage: missing coverage between %d and %d", maxValue, globalRangeMax))
+	}
+	if globalRangeMin > minValue {
+		errs = append(errs, fmt.Errorf("incomplete coverage: missing coverage between %d and %d", globalRangeMin, minValue))
+	}
+
+	return errs
+}
+
+func validateBooleanThresholds(thresholds []*rpc.BooleanThreshold) []error {
+	errs := make([]error, 0)
+
+	var isFalseCovered, isTrueCovered bool
+	for _, t := range thresholds {
+		if isFalseCovered && !t.GetValue() {
+			errs = append(errs, fmt.Errorf("duplicate entries for 'false' value"))
+		}
+
+		if isTrueCovered && t.GetValue() {
+			errs = append(errs, fmt.Errorf("duplicate entries for 'true' value"))
+		}
+
+		isFalseCovered = !t.GetValue() || isFalseCovered
+		isTrueCovered = t.GetValue() || isTrueCovered
+	}
+
+	if !isTrueCovered || !isFalseCovered {
+		errs = append(errs, fmt.Errorf("missing coverage for one or both of the boolean values"))
+	}
+	return errs
+}


Test patch:
diff --git a/cmd/registry/cmd/upload/artifact_test.go b/cmd/registry/cmd/upload/artifact_test.go
index 4c74f7b..545a205 100644
--- a/cmd/registry/cmd/upload/artifact_test.go
+++ b/cmd/registry/cmd/upload/artifact_test.go
@@ -163,17 +163,17 @@ func TestScoreDefinitionArtifactUpload(t *testing.T) {
 						MaxValue: 100,
 						Thresholds: []*rpc.NumberThreshold{
 							{
-								Severity: rpc.Severity_ALERT,
+								Severity: rpc.Severity_OK,
 								Range: &rpc.NumberThreshold_NumberRange{
-									Min: 60,
-									Max: 100,
+									Min: 0,
+									Max: 59,
 								},
 							},
 							{
-								Severity: rpc.Severity_OK,
+								Severity: rpc.Severity_ALERT,
 								Range: &rpc.NumberThreshold_NumberRange{
-									Min: 0,
-									Max: 59,
+									Min: 60,
+									Max: 100,
 								},
 							},
 						},
diff --git a/cmd/registry/patterns/parser_test.go b/cmd/registry/patterns/parser_test.go
index e8dc781..eee2653 100644
--- a/cmd/registry/patterns/parser_test.go
+++ b/cmd/registry/patterns/parser_test.go
@@ -33,7 +33,7 @@ func generateArtifact(t *testing.T, artifactName string) names.Artifact {
 	return artifact
 }
 
-func TestSubstitueReferenceEntity(t *testing.T) {
+func TestSubstituteReferenceEntity(t *testing.T) {
 	tests := []struct {
 		desc              string
 		resourcePattern   string
@@ -72,25 +72,24 @@ func TestSubstitueReferenceEntity(t *testing.T) {
 		},
 	}
 
-	const projectID = "demo"
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
 			resourceName, err := ParseResourcePattern(test.resourcePattern)
 			if err != nil {
 				t.Fatalf("Error in parsing, %s", err)
 			}
-			got, err := SubstituteReferenceEntity(test.dependencyPattern, resourceName, projectID)
+			got, err := SubstituteReferenceEntity(test.dependencyPattern, resourceName)
 			if err != nil {
 				t.Errorf("SubstituteReferenceEntity returned unexpected error: %s", err)
 			}
-			if got != test.want {
+			if got.String() != test.want {
 				t.Errorf("SubstituteReferenceEntity returned unexpected value want: %q got:%q", test.want, got)
 			}
 		})
 	}
 }
 
-func TestSubstitueReferenceEntityError(t *testing.T) {
+func TestSubstituteReferenceEntityError(t *testing.T) {
 	tests := []struct {
 		desc              string
 		resourcePattern   string
@@ -108,16 +107,15 @@ func TestSubstitueReferenceEntityError(t *testing.T) {
 		},
 	}
 
-	const projectID = "demo"
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
 			resourceName, err := ParseResourcePattern(test.resourcePattern)
 			if err != nil {
 				t.Fatalf("Error in parsing, %s", err)
 			}
-			got, err := SubstituteReferenceEntity(test.dependencyPattern, resourceName, projectID)
+			got, err := SubstituteReferenceEntity(test.dependencyPattern, resourceName)
 			if err == nil {
-				t.Errorf("expected SubstituteReferenceEntity to return error, got: %q", got)
+				t.Errorf("expected SubstituteReferenceEntity to return error, got: %q", got.String())
 			}
 		})
 	}
diff --git a/cmd/registry/scoring/parser_test.go b/cmd/registry/scoring/parser_test.go
new file mode 100644
index 0000000..1e52aa0
--- /dev/null
+++ b/cmd/registry/scoring/parser_test.go
@@ -0,0 +1,1391 @@
+package scoring
+
+import (
+	"context"
+	"testing"
+
+	"github.com/apigee/registry/cmd/registry/patterns"
+	"github.com/apigee/registry/rpc"
+)
+
+func TestValidateScoreDefinition(t *testing.T) {
+	tests := []struct {
+		desc            string
+		parent          string
+		scoreDefinition *rpc.ScoreDefinition
+		wantNumErr      int
+	}{
+		// No errors
+		{
+			desc:   "score formula",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				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_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 61,
+									Max: 100,
+								},
+							},
+							{
+								Severity: rpc.Severity_WARNING,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 0,
+									Max: 60,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			desc:   "rollup formula",
+			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",
+							},
+						},
+						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,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			desc:   "no thresholds",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				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,
+					},
+				},
+			},
+		},
+		{
+			desc:   "only max limit",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				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{
+						MaxValue: 100,
+					},
+				},
+			},
+		},
+		// Single errors
+		{
+			desc:   "target pattern error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/specs/-", //error
+				},
+				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,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:   "score formula error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.artifact/conformance-report", //error
+						},
+						ScoreExpression: "count(errors)",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:   "rollup formula error",
+			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.artifact/conformance-report", //error
+								},
+								ScoreExpression: "count(warnings)",
+								ReferenceId:     "lint_warnings",
+							},
+						},
+						RollupExpression: "lint_errors+lint_warnings",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:   "percent threshold error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/conformance-report",
+						},
+						ScoreExpression: "count(errors)",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Percent{
+					Percent: &rpc.PercentType{
+						Thresholds: []*rpc.NumberThreshold{
+							{
+								Severity: rpc.Severity_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{ //error
+									Min: 60,
+									Max: 100,
+								},
+							},
+							{
+								Severity: rpc.Severity_WARNING,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 0,
+									Max: 60,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:   "integer threshold error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				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_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{ //error
+									Min: 62,
+									Max: 100,
+								},
+							},
+							{
+								Severity: rpc.Severity_WARNING,
+								Range: &rpc.NumberThreshold_NumberRange{
+									Min: 0,
+									Max: 60,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:   "integer threshold no limits",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				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{
+						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:   "boolean threshold error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/audit-report",
+						},
+						ScoreExpression: "isApproved",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Boolean{
+					Boolean: &rpc.BooleanType{
+						Thresholds: []*rpc.BooleanThreshold{ //error
+							{
+								Severity: rpc.Severity_ALERT,
+								Value:    false,
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		// Combination errors
+		{
+			desc:   "target pattern and score formula error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/specs/-", //error
+				},
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.specs/artifacts/conformance-report", //error
+						},
+						ScoreExpression: "count(errors)",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+					},
+				},
+			},
+			// This is expected here since it is not possible to validate ScoreFormula patterns if there are errors in the targetResource pattern.
+			wantNumErr: 1,
+		},
+		{
+			desc:   "target pattern and threshold error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/specs/-", //error
+				},
+				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_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{ //error
+									Min: 60,
+									Max: 100,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:   "score formula and threshold error",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.specs/artifacts/conformance-report", //error
+						},
+						ScoreExpression: "count(errors)",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+						Thresholds: []*rpc.NumberThreshold{
+							{
+								Severity: rpc.Severity_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{ //error
+									Min: 60,
+									Max: 100,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:   "error in each component",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/specs/-", //error
+				},
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.specs/artifacts/conformance-report", //error
+						},
+						ScoreExpression: "count(errors)",
+					},
+				},
+				Type: &rpc.ScoreDefinition_Integer{
+					Integer: &rpc.IntegerType{
+						MinValue: 0,
+						MaxValue: 100,
+						Thresholds: []*rpc.NumberThreshold{
+							{
+								Severity: rpc.Severity_ALERT,
+								Range: &rpc.NumberThreshold_NumberRange{ //error
+									Min: 60,
+									Max: 100,
+								},
+							},
+						},
+					},
+				},
+			},
+			// 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
+		{
+			desc:   "missing formula",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/specs/-", //error
+				},
+				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 type",
+			parent: "projects/demo/locations/global",
+			scoreDefinition: &rpc.ScoreDefinition{
+				Id:   "test-score-definition",
+				Kind: "ScoreDefinition",
+				TargetResource: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-",
+				},
+				Formula: &rpc.ScoreDefinition_ScoreFormula{
+					ScoreFormula: &rpc.ScoreFormula{
+						Artifact: &rpc.ResourcePattern{
+							Pattern: "$resource.spec/artifacts/conformance-report",
+						},
+						ScoreExpression: "count(errors)",
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			ctx := context.Background()
+			gotErrs := ValidateScoreDefinition(ctx, test.parent, test.scoreDefinition)
+			if len(gotErrs) != test.wantNumErr {
+				t.Errorf("ValidateScoreDefinition() returned unexpected no. of errors: want %d, got %s", test.wantNumErr, gotErrs)
+			}
+		})
+	}
+}
+
+func TestValidateScoreFormula(t *testing.T) {
+	tests := []struct {
+		desc          string
+		targetPattern *rpc.ResourcePattern
+		scoreFormula  *rpc.ScoreFormula
+		wantNumErr    int
+	}{
+		// No errors
+		{
+			desc: "score formula",
+			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)",
+			},
+		},
+		// Single errors
+		{
+			desc: "no $resource reference",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-/artifacts/conformance-report", //error
+				},
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc: "invalid $resource reference",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.specs/artifacts/conformance-report", //error
+				},
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc: "invalid $resource wrt targetName",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/conformance-report", //error
+				},
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc: "missing artifact name",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/-", //error
+				},
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 1,
+		},
+		// Combination errors
+		{
+			desc: "invalid pattern and missing name",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.specs/-/artifacts/-", //error
+				},
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc: "no reference and missing name",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-/specs/-",
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "apis/-/versions/-/specs/-/artifacts/-", //error
+				},
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc: "invalid reference and missing name",
+			targetPattern: &rpc.ResourcePattern{
+				Pattern: "projects/demo/locations/global/apis/-/versions/-", //error
+			},
+			scoreFormula: &rpc.ScoreFormula{
+				Artifact: &rpc.ResourcePattern{
+					Pattern: "$resource.spec/artifacts/-",
+				},
+				ScoreExpression: "count(errors)",
+			},
+			wantNumErr: 2,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			targetName, _ := patterns.ParseResourcePattern(test.targetPattern.GetPattern())
+			gotErrs := validateScoreFormula(targetName, test.scoreFormula)
+			if len(gotErrs) != test.wantNumErr {
+				t.Errorf("validateScoreFormula() returned unexpected no. of errors: want %d, got %s", test.wantNumErr, gotErrs)
+			}
+		})
+	}
+}
+
+func TestValidateNumberThresholds(t *testing.T) {
+	tests := []struct {
+		desc       string
+		minValue   int32
+		maxValue   int32
+		thresholds []*rpc.NumberThreshold
+		wantNumErr int
+	}{
+		// no errors
+		{
+			desc:     "percentage thresholds",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 31,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 61,
+						Max: 100,
+					},
+				},
+			},
+		},
+		{
+			desc:     "integer thresholds",
+			minValue: -50,
+			maxValue: 50,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: -50,
+						Max: -20,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: -19,
+						Max: 10,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 11,
+						Max: 50,
+					},
+				},
+			},
+		},
+		{
+			desc:     "descending score thresholds",
+			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: 31,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 30,
+					},
+				},
+			},
+		},
+		{
+			desc:     "distributed severity",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 10,
+					},
+				},
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 91,
+						Max: 100,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 11,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 71,
+						Max: 90,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 31,
+						Max: 70,
+					},
+				},
+			},
+		},
+		// single errors
+		{
+			desc:     "missing range",
+			minValue: -50,
+			maxValue: 50,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:     "range.min greater than range.max",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Max: 0,
+						Min: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 51,
+						Max: 100,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:     "out of minValue bound",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: -1,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 31,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 61,
+						Max: 100,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:     "out of maxValue bound",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 31,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 61,
+						Max: 101,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:     "missing coverage for minValue",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 5,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 31,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 61,
+						Max: 100,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:     "missing coverage for maxValue",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 31,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 61,
+						Max: 90,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:     "missing coverage in between",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 29,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 31,
+						Max: 59,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 61,
+						Max: 100,
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:     "overlap",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 30,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 60,
+						Max: 100,
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		// Combination errors
+		{
+			desc:     "out of min and max limits",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: -1,
+						Max: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 51,
+						Max: 101,
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:     "out of limits and overlap",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: -1,
+						Max: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 50,
+						Max: 101,
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:     "out of limits and missing coverage",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: -1,
+						Max: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 52,
+						Max: 101,
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:     "missing limits coverage",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 2,
+						Max: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 51,
+						Max: 99,
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:     "missing limits coverage and overlap",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 2,
+						Max: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 50,
+						Max: 99,
+					},
+				},
+			},
+			wantNumErr: 3,
+		},
+		{
+			desc:     "missing limits coverage and missing coverage",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 2,
+						Max: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 52,
+						Max: 99,
+					},
+				},
+			},
+			wantNumErr: 3,
+		},
+		{
+			desc:     "missing min coverage and out of max limit",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 2,
+						Max: 50,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 51,
+						Max: 101,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc:     "overlap and missing coverage",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 30,
+					},
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 30,
+						Max: 60,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 62,
+						Max: 100,
+					},
+				},
+			},
+			wantNumErr: 2,
+		},
+		{
+			desc:     "nested thresholds",
+			minValue: 0,
+			maxValue: 100,
+			thresholds: []*rpc.NumberThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 0,
+						Max: 100,
+					},
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Range: &rpc.NumberThreshold_NumberRange{
+						Min: 20,
+						Max: 50,
+					},
+				},
+			},
+			wantNumErr: 1,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			gotErrs := validateNumberThresholds(test.thresholds, test.minValue, test.maxValue)
+			if len(gotErrs) != test.wantNumErr {
+				t.Errorf("validateNumberThresholds() returned unexpected no. of errors: want %d, got %s", test.wantNumErr, gotErrs)
+			}
+		})
+	}
+
+}
+
+func TestValidateBooleanThresholds(t *testing.T) {
+	tests := []struct {
+		desc       string
+		thresholds []*rpc.BooleanThreshold
+		wantNumErr int
+	}{
+		// no errors
+		{
+			desc: "normal case",
+			thresholds: []*rpc.BooleanThreshold{
+				{
+					Severity: rpc.Severity_ALERT,
+					Value:    false,
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Value:    true,
+				},
+			},
+		},
+		{
+			desc: "same severity",
+			thresholds: []*rpc.BooleanThreshold{
+				{
+					Severity: rpc.Severity_OK,
+					Value:    false,
+				},
+				{
+					Severity: rpc.Severity_OK,
+					Value:    true,
+				},
+			},
+		},
+		// single errors
+		{
+			desc: "missing false",
+			thresholds: []*rpc.BooleanThreshold{
+				{
+					Severity: rpc.Severity_OK,
+					Value:    true,
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc: "missing true",
+			thresholds: []*rpc.BooleanThreshold{
+				{
+					Severity: rpc.Severity_WARNING,
+					Value:    false,
+				},
+			},
+			wantNumErr: 1,
+		},
+		{
+			desc: "duplicate entries",
+			thresholds: []*rpc.BooleanThreshold{
+				{
+					Severity: rpc.Severity_OK,
+					Value:    true,
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Value:    true,
+				},
+				{
+					Severity: rpc.Severity_ALERT,
+					Value:    false,
+				},
+			},
+			wantNumErr: 1,
+		},
+		// combination errors
+		{
+			desc: "missing and duplicate entries",
+			thresholds: []*rpc.BooleanThreshold{
+				{
+					Severity: rpc.Severity_OK,
+					Value:    true,
+				},
+				{
+					Severity: rpc.Severity_WARNING,
+					Value:    true,
+				},
+			},
+			wantNumErr: 2,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			gotErrs := validateBooleanThresholds(test.thresholds)
+			if len(gotErrs) != test.wantNumErr {
+				t.Errorf("validateBooleanThresholds() returned unexpected no. of errors: want %d, got %s", test.wantNumErr, gotErrs)
+			}
+		})
+	}
+}