Repository: apigee/registry
Pull request #678 (2022-07-27 17:23:13)

Replace filtering.Field type with maps

codecov[bot]: # [Codecov](https://codecov.io/gh/apigee/registry/pull/678?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) Report
> Merging [#678](https://codecov.io/gh/apigee/registry/pull/678?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (9773a54) into [main](https://codecov.io/gh/apigee/registry/commit/dfc318924c6e3c1a3efed35c80c332cdac764b49?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (dfc3189) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##             main     #678   +/-   ##
=======================================
  Coverage   54.09%   54.09%           
=======================================
  Files          78       78           
  Lines        7326     7326           
=======================================
  Hits         3963     3963           
  Misses       2940     2940           
  Partials      423      423           
```


| [Impacted Files](https://codecov.io/gh/apigee/registry/pull/678?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) | Coverage Δ | |
|---|---|---|
| [...r/registry/internal/storage/filtering/filtering.go](https://codecov.io/gh/apigee/registry/pull/678/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-c2VydmVyL3JlZ2lzdHJ5L2ludGVybmFsL3N0b3JhZ2UvZmlsdGVyaW5nL2ZpbHRlcmluZy5nbw==) | `86.04% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee).


Patch:
diff --git a/server/registry/internal/storage/filtering/filtering.go b/server/registry/internal/storage/filtering/filtering.go
index add5598..8f79fcf 100644
--- a/server/registry/internal/storage/filtering/filtering.go
+++ b/server/registry/internal/storage/filtering/filtering.go
@@ -33,11 +33,6 @@ const (
 	StringMap FieldType = iota
 )
 
-type Field struct {
-	Name string
-	Type FieldType
-}
-
 type Filter struct {
 	program cel.Program
 }
@@ -60,22 +55,22 @@ func (f *Filter) Matches(model map[string]interface{}) (bool, error) {
 	return match, nil
 }
 
-func NewFilter(filter string, fields []Field) (Filter, error) {
+func NewFilter(filter string, fields map[string]FieldType) (Filter, error) {
 	if filter == "" {
 		return Filter{}, nil
 	}
 
 	declarations := make([]*exprpb.Decl, 0)
-	for _, field := range fields {
-		switch field.Type {
+	for name, fieldType := range fields {
+		switch fieldType {
 		case String:
-			declarations = append(declarations, decls.NewConst(field.Name, decls.String, nil))
+			declarations = append(declarations, decls.NewConst(name, decls.String, nil))
 		case Int:
-			declarations = append(declarations, decls.NewConst(field.Name, decls.Int, nil))
+			declarations = append(declarations, decls.NewConst(name, decls.Int, nil))
 		case Timestamp:
-			declarations = append(declarations, decls.NewConst(field.Name, decls.Timestamp, nil))
+			declarations = append(declarations, decls.NewConst(name, decls.Timestamp, nil))
 		case StringMap:
-			declarations = append(declarations, decls.NewConst(field.Name, decls.NewMapType(decls.String, decls.String), nil))
+			declarations = append(declarations, decls.NewConst(name, decls.NewMapType(decls.String, decls.String), nil))
 		default:
 			return Filter{}, status.Errorf(codes.InvalidArgument, "unknown filter argument type")
 		}
diff --git a/server/registry/internal/storage/list.go b/server/registry/internal/storage/list.go
index f12aeb5..de9bee9 100644
--- a/server/registry/internal/storage/list.go
+++ b/server/registry/internal/storage/list.go
@@ -25,6 +25,90 @@ import (
 	"gorm.io/gorm"
 )
 
+var projectFields = map[string]filtering.FieldType{
+	"name":         filtering.String,
+	"project_id":   filtering.String,
+	"display_name": filtering.String,
+	"description":  filtering.String,
+	"create_time":  filtering.Timestamp,
+	"update_time":  filtering.Timestamp,
+}
+
+var apiFields = map[string]filtering.FieldType{
+	"name":                   filtering.String,
+	"project_id":             filtering.String,
+	"api_id":                 filtering.String,
+	"display_name":           filtering.String,
+	"description":            filtering.String,
+	"create_time":            filtering.Timestamp,
+	"update_time":            filtering.Timestamp,
+	"availability":           filtering.String,
+	"recommended_version":    filtering.String,
+	"recommended_deployment": filtering.String,
+	"labels":                 filtering.StringMap,
+}
+
+var versionFields = map[string]filtering.FieldType{
+	"name":         filtering.String,
+	"project_id":   filtering.String,
+	"api_id":       filtering.String,
+	"version_id":   filtering.String,
+	"display_name": filtering.String,
+	"description":  filtering.String,
+	"create_time":  filtering.Timestamp,
+	"update_time":  filtering.Timestamp,
+	"state":        filtering.String,
+	"labels":       filtering.StringMap,
+}
+
+var specFields = map[string]filtering.FieldType{
+	"name":                 filtering.String,
+	"project_id":           filtering.String,
+	"api_id":               filtering.String,
+	"version_id":           filtering.String,
+	"spec_id":              filtering.String,
+	"filename":             filtering.String,
+	"description":          filtering.String,
+	"create_time":          filtering.Timestamp,
+	"revision_create_time": filtering.Timestamp,
+	"revision_update_time": filtering.Timestamp,
+	"mime_type":            filtering.String,
+	"size_bytes":           filtering.Int,
+	"source_uri":           filtering.String,
+	"labels":               filtering.StringMap,
+}
+
+var deploymentFields = map[string]filtering.FieldType{
+	"name":                 filtering.String,
+	"project_id":           filtering.String,
+	"api_id":               filtering.String,
+	"deployment_id":        filtering.String,
+	"display_name":         filtering.String,
+	"description":          filtering.String,
+	"create_time":          filtering.Timestamp,
+	"revision_create_time": filtering.Timestamp,
+	"revision_update_time": filtering.Timestamp,
+	"api_spec_revision":    filtering.String,
+	"endpoint_uri":         filtering.String,
+	"external_channel_uri": filtering.String,
+	"intended_audience":    filtering.String,
+	"access_guidance":      filtering.String,
+	"labels":               filtering.StringMap,
+}
+
+var artifactFields = map[string]filtering.FieldType{
+	"name":        filtering.String,
+	"project_id":  filtering.String,
+	"api_id":      filtering.String,
+	"version_id":  filtering.String,
+	"spec_id":     filtering.String,
+	"artifact_id": filtering.String,
+	"create_time": filtering.Timestamp,
+	"update_time": filtering.Timestamp,
+	"mime_type":   filtering.String,
+	"size_bytes":  filtering.Int,
+}
+
 // limit returns the database page size to use for a listing request.
 func limit(opts PageOptions) int {
 	// Without filters, read exactly enough rows to fill the page,
@@ -43,15 +127,6 @@ type ProjectList struct {
 	Token    string
 }
 
-var projectFields = []filtering.Field{
-	{Name: "name", Type: filtering.String},
-	{Name: "project_id", Type: filtering.String},
-	{Name: "display_name", Type: filtering.String},
-	{Name: "description", Type: filtering.String},
-	{Name: "create_time", Type: filtering.Timestamp},
-	{Name: "update_time", Type: filtering.Timestamp},
-}
-
 func (c *Client) ListProjects(ctx context.Context, opts PageOptions) (ProjectList, error) {
 	token, err := decodeToken(opts.Token)
 	if err != nil {
@@ -126,20 +201,6 @@ type ApiList struct {
 	Token string
 }
 
-var apiFields = []filtering.Field{
-	{Name: "name", Type: filtering.String},
-	{Name: "project_id", Type: filtering.String},
-	{Name: "api_id", Type: filtering.String},
-	{Name: "display_name", Type: filtering.String},
-	{Name: "description", Type: filtering.String},
-	{Name: "create_time", Type: filtering.Timestamp},
-	{Name: "update_time", Type: filtering.Timestamp},
-	{Name: "availability", Type: filtering.String},
-	{Name: "recommended_version", Type: filtering.String},
-	{Name: "recommended_deployment", Type: filtering.String},
-	{Name: "labels", Type: filtering.StringMap},
-}
-
 func (c *Client) ListApis(ctx context.Context, parent names.Project, opts PageOptions) (ApiList, error) {
 	token, err := decodeToken(opts.Token)
 	if err != nil {
@@ -238,19 +299,6 @@ type VersionList struct {
 	Token    string
 }
 
-var versionFields = []filtering.Field{
-	{Name: "name", Type: filtering.String},
-	{Name: "project_id", Type: filtering.String},
-	{Name: "api_id", Type: filtering.String},
-	{Name: "version_id", Type: filtering.String},
-	{Name: "display_name", Type: filtering.String},
-	{Name: "description", Type: filtering.String},
-	{Name: "create_time", Type: filtering.Timestamp},
-	{Name: "update_time", Type: filtering.Timestamp},
-	{Name: "state", Type: filtering.String},
-	{Name: "labels", Type: filtering.StringMap},
-}
-
 func (c *Client) ListVersions(ctx context.Context, parent names.Api, opts PageOptions) (VersionList, error) {
 	token, err := decodeToken(opts.Token)
 	if err != nil {
@@ -358,23 +406,6 @@ type SpecList struct {
 	Token string
 }
 
-var specFields = []filtering.Field{
-	{Name: "name", Type: filtering.String},
-	{Name: "project_id", Type: filtering.String},
-	{Name: "api_id", Type: filtering.String},
-	{Name: "version_id", Type: filtering.String},
-	{Name: "spec_id", Type: filtering.String},
-	{Name: "filename", Type: filtering.String},
-	{Name: "description", Type: filtering.String},
-	{Name: "create_time", Type: filtering.Timestamp},
-	{Name: "revision_create_time", Type: filtering.Timestamp},
-	{Name: "revision_update_time", Type: filtering.Timestamp},
-	{Name: "mime_type", Type: filtering.String},
-	{Name: "size_bytes", Type: filtering.Int},
-	{Name: "source_uri", Type: filtering.String},
-	{Name: "labels", Type: filtering.StringMap},
-}
-
 func (c *Client) ListSpecs(ctx context.Context, parent names.Version, opts PageOptions) (SpecList, error) {
 	token, err := decodeToken(opts.Token)
 	if err != nil {
@@ -573,24 +604,6 @@ type DeploymentList struct {
 	Token       string
 }
 
-var deploymentFields = []filtering.Field{
-	{Name: "name", Type: filtering.String},
-	{Name: "project_id", Type: filtering.String},
-	{Name: "api_id", Type: filtering.String},
-	{Name: "deployment_id", Type: filtering.String},
-	{Name: "display_name", Type: filtering.String},
-	{Name: "description", Type: filtering.String},
-	{Name: "create_time", Type: filtering.Timestamp},
-	{Name: "revision_create_time", Type: filtering.Timestamp},
-	{Name: "revision_update_time", Type: filtering.Timestamp},
-	{Name: "api_spec_revision", Type: filtering.String},
-	{Name: "endpoint_uri", Type: filtering.String},
-	{Name: "external_channel_uri", Type: filtering.String},
-	{Name: "intended_audience", Type: filtering.String},
-	{Name: "access_guidance", Type: filtering.String},
-	{Name: "labels", Type: filtering.StringMap},
-}
-
 func (c *Client) ListDeployments(ctx context.Context, parent names.Api, opts PageOptions) (DeploymentList, error) {
 	token, err := decodeToken(opts.Token)
 	if err != nil {
@@ -774,19 +787,6 @@ type ArtifactList struct {
 	Token     string
 }
 
-var artifactFields = []filtering.Field{
-	{Name: "name", Type: filtering.String},
-	{Name: "project_id", Type: filtering.String},
-	{Name: "api_id", Type: filtering.String},
-	{Name: "version_id", Type: filtering.String},
-	{Name: "spec_id", Type: filtering.String},
-	{Name: "artifact_id", Type: filtering.String},
-	{Name: "create_time", Type: filtering.Timestamp},
-	{Name: "update_time", Type: filtering.Timestamp},
-	{Name: "mime_type", Type: filtering.String},
-	{Name: "size_bytes", Type: filtering.Int},
-}
-
 func (c *Client) ListSpecArtifacts(ctx context.Context, parent names.Spec, opts PageOptions) (ArtifactList, error) {
 	token, err := decodeToken(opts.Token)
 	if err != nil {


Test patch:
diff --git a/server/registry/internal/storage/filtering/filtering_test.go b/server/registry/internal/storage/filtering/filtering_test.go
index df62ba8..3bb2c74 100644
--- a/server/registry/internal/storage/filtering/filtering_test.go
+++ b/server/registry/internal/storage/filtering/filtering_test.go
@@ -25,32 +25,33 @@ func TestErrorConditions(t *testing.T) {
 	tests := []struct {
 		desc   string
 		filter string
-		fields []Field
+		fields map[string]FieldType
 		model  map[string]interface{}
 	}{
 		{
 			desc:   "bad field type",
 			filter: `k == "match"`,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: 999,
-				},
+			fields: map[string]FieldType{
+				"k": 999,
+			},
+		},
+		{
+			desc:   "bad field name",
+			filter: `abc == "match"`,
+			fields: map[string]FieldType{
+				"k": 999,
 			},
 		},
 		{
 			desc:   "bad filter",
 			filter: `k xx "match"`,
-			fields: []Field{},
+			fields: map[string]FieldType{},
 		},
 		{
 			desc:   "bad filter result",
 			filter: `k`,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: String,
-				},
+			fields: map[string]FieldType{
+				"k": String,
 			},
 			model: map[string]interface{}{
 				"k": "match",
@@ -59,11 +60,8 @@ func TestErrorConditions(t *testing.T) {
 		{
 			desc:   "bad model",
 			filter: `k == "k"`,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: String,
-				},
+			fields: map[string]FieldType{
+				"k": String,
 			},
 			model: nil,
 		},
@@ -88,18 +86,15 @@ func TestFilter_Matches(t *testing.T) {
 	tests := []struct {
 		desc     string
 		filter   string
-		fields   []Field
+		fields   map[string]FieldType
 		positive map[string]interface{}
 		negative map[string]interface{}
 	}{
 		{
 			desc:   "empty",
 			filter: ``,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: String,
-				},
+			fields: map[string]FieldType{
+				"k": String,
 			},
 			positive: map[string]interface{}{
 				"k": "match",
@@ -109,11 +104,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "equal to String",
 			filter: `k == "match"`,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: String,
-				},
+			fields: map[string]FieldType{
+				"k": String,
 			},
 			positive: map[string]interface{}{
 				"k": "match",
@@ -125,11 +117,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "equal to Int",
 			filter: `k == 123`,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: Int,
-				},
+			fields: map[string]FieldType{
+				"k": Int,
 			},
 			positive: map[string]interface{}{
 				"k": 123,
@@ -141,11 +130,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "less than Timestamp",
 			filter: `k < timestamp("2021-01-01T00:00:00Z")`,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: Timestamp,
-				},
+			fields: map[string]FieldType{
+				"k": Timestamp,
 			},
 			positive: map[string]interface{}{
 				"k": time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC),
@@ -157,11 +143,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "greater than Timestamp",
 			filter: `k > timestamp("2021-01-01T00:00:00Z")`,
-			fields: []Field{
-				{
-					Name: "k",
-					Type: Timestamp,
-				},
+			fields: map[string]FieldType{
+				"k": Timestamp,
 			},
 			positive: map[string]interface{}{
 				"k": time.Date(2022, time.January, 1, 0, 0, 0, 0, time.UTC),
@@ -173,11 +156,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "has StringMap key",
 			filter: `has(labels.match)`,
-			fields: []Field{
-				{
-					Name: "labels",
-					Type: StringMap,
-				},
+			fields: map[string]FieldType{
+				"labels": StringMap,
 			},
 			positive: map[string]interface{}{
 				"labels": map[string]string{
@@ -193,11 +173,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "in StringMap keys",
 			filter: `"match" in labels`,
-			fields: []Field{
-				{
-					Name: "labels",
-					Type: StringMap,
-				},
+			fields: map[string]FieldType{
+				"labels": StringMap,
 			},
 			positive: map[string]interface{}{
 				"labels": map[string]string{
@@ -213,11 +190,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "equal to StringMap value",
 			filter: `labels["k"] == "match"`,
-			fields: []Field{
-				{
-					Name: "labels",
-					Type: StringMap,
-				},
+			fields: map[string]FieldType{
+				"labels": StringMap,
 			},
 			positive: map[string]interface{}{
 				"labels": map[string]string{
@@ -233,11 +207,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "substring of StringMap value",
 			filter: `labels.k.contains("substring")`,
-			fields: []Field{
-				{
-					Name: "labels",
-					Type: StringMap,
-				},
+			fields: map[string]FieldType{
+				"labels": StringMap,
 			},
 			positive: map[string]interface{}{
 				"labels": map[string]string{
@@ -253,11 +224,8 @@ func TestFilter_Matches(t *testing.T) {
 		{
 			desc:   "in StringMap value split",
 			filter: `"match" in labels.k.split("_")`,
-			fields: []Field{
-				{
-					Name: "labels",
-					Type: StringMap,
-				},
+			fields: map[string]FieldType{
+				"labels": StringMap,
 			},
 			positive: map[string]interface{}{
 				"labels": map[string]string{