Skip to content

Commit

Permalink
add registry rules for Api (#935)
Browse files Browse the repository at this point in the history
  • Loading branch information
theganyo authored Jan 13, 2023
1 parent aeaeb36 commit fd1247b
Show file tree
Hide file tree
Showing 16 changed files with 437 additions and 89 deletions.
14 changes: 14 additions & 0 deletions cmd/registry/cmd/check/lint/problem.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@ const (
ERROR
)

func (s Severity) String() string {
switch s {
case INFO:
return "INFO"
case WARNING:
return "WARNING"
case ERROR:
return "ERROR"
}
return "UNKNOWN"
}

// Problem contains information about a result produced by an API Linter.
//
// All rules return []Problem. Most lint rules return 0 or 1 problems, but
Expand Down Expand Up @@ -71,12 +83,14 @@ func (p Problem) marshal() interface{} {
Location string `json:"location" yaml:"location"`
RuleID RuleName `json:"rule_id" yaml:"rule_id"`
RuleDocURI string `json:"rule_doc_uri" yaml:"rule_doc_uri"`
Severity string `json:"severity" yaml:"severity"`
}{
p.Message,
p.Suggestion,
p.Location,
p.RuleID,
p.GetRuleURI(),
p.Severity.String(),
}
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/registry/cmd/check/lint/problem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func TestProblemYAML(t *testing.T) {
Message: "foo bar",
Location: "test/location",
RuleID: "core::0131",
Severity: ERROR,
}
serialized, err := yaml.Marshal(problem)
if err != nil {
Expand All @@ -66,6 +67,7 @@ func TestProblemYAML(t *testing.T) {
{"Message", `message: foo bar`},
{"Location", `location: test/location`},
{"RuleID", `rule_id: core::0131`},
{"Severity", `ERROR`},
}
for _, test := range tests {
t.Run(test.testName, func(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions cmd/registry/cmd/check/lint/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import (

// Response collects the results of running the Rules.
type Response struct {
sync.Mutex
RunTime time.Time `json:"time" yaml:"time"`
Problems []Problem `json:"problems" yaml:"problems"`
Error error `json:"error,omitempty" yaml:"error,omitempty"` // populated if panic
sync.Mutex `json:"-" yaml:"-"`
RunTime time.Time `json:"time" yaml:"time"`
Problems []Problem `json:"problems" yaml:"problems"`
Error error `json:"error,omitempty" yaml:"error,omitempty"` // populated if panic
}

func (r *Response) append(res Resource, probs []Problem, err error) {
Expand Down
17 changes: 1 addition & 16 deletions cmd/registry/cmd/check/lint/rule_groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import "fmt"
var ruleGroup = []func(int) string{
registryGroup,
hubGroup,
controllerGroup,
scoreGroup,
styleGroup,
}

func registryGroup(ruleNum int) string {
Expand All @@ -28,25 +26,12 @@ func hubGroup(ruleNum int) string {
return ""
}

func controllerGroup(ruleNum int) string {
if ruleNum >= 1100 && ruleNum < 1200 {
return "controller"
}
return ""
}

func scoreGroup(ruleNum int) string {
if ruleNum >= 1200 && ruleNum < 1300 {
if ruleNum >= 1100 && ruleNum < 1200 {
return "score"
}
return ""
}
func styleGroup(ruleNum int) string {
if ruleNum >= 1300 && ruleNum < 1400 {
return "style"
}
return ""
}

// getRuleGroup takes an rule number and returns the appropriate group.
// It panics if no group is found.
Expand Down
8 changes: 2 additions & 6 deletions cmd/registry/cmd/check/lint/rule_groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,8 @@ func TestGroups(t *testing.T) {
{"registry", 999, "registry"},
{"hub", 1000, "hub"},
{"hub", 1099, "hub"},
{"controller", 1100, "controller"},
{"controller", 1199, "controller"},
{"score", 1200, "score"},
{"score", 1299, "score"},
{"style", 1300, "style"},
{"style", 1399, "style"},
{"score", 1100, "score"},
{"score", 1199, "score"},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
Expand Down
26 changes: 0 additions & 26 deletions cmd/registry/cmd/check/rules/rule0001/rule0001.go

This file was deleted.

27 changes: 0 additions & 27 deletions cmd/registry/cmd/check/rules/rule0001/rule0001_test.go

This file was deleted.

53 changes: 53 additions & 0 deletions cmd/registry/cmd/check/rules/rule100/rule100.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2023 Google LLC. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Api availability field is free-form, but we expect single words that describe availability.
// https://github.com/apigee/registry/blob/main/google/cloud/apigeeregistry/v1/registry_models.proto#L51
package rule100

import (
"strings"

"github.com/apigee/registry/cmd/registry/cmd/check/lint"
"github.com/apigee/registry/rpc"
)

var ruleNum = 100
var ruleName = lint.NewRuleName(ruleNum, "api-availability-single-word")

// AddRules accepts a register function and registers each of
// this rules' checks to the RuleRegistry.
func AddRules(r lint.RuleRegistry) error {
return r.Register(
ruleNum,
availabilitySingleWord,
)
}

var availabilitySingleWord = &lint.ApiRule{
Name: ruleName,
OnlyIf: func(a *rpc.Api) bool {
return true
},
ApplyToApi: func(a *rpc.Api) []lint.Problem {
if arr := strings.SplitN(a.Availability, " ", 2); len(arr) > 1 {
return []lint.Problem{{
Severity: lint.INFO,
Message: `Availability is free-form, but we expect single words that describe availability.`,
Suggestion: `Use single words like: "NONE", "TESTING", "PREVIEW", "GENERAL", "DEPRECATED", "SHUTDOWN"`,
}}
}
return nil
},
}
56 changes: 56 additions & 0 deletions cmd/registry/cmd/check/rules/rule100/rule100_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2023 Google LLC. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package rule100

import (
"testing"

"github.com/apigee/registry/cmd/registry/cmd/check/lint"
"github.com/apigee/registry/rpc"
"github.com/google/go-cmp/cmp"
)

func TestAddRules(t *testing.T) {
if err := AddRules(lint.NewRuleRegistry()); err != nil {
t.Errorf("AddRules got an error: %v", err)
}
}

func Test_availabilitySingleWord(t *testing.T) {
prob := []lint.Problem{{
Severity: lint.INFO,
Message: `Availability is free-form, but we expect single words that describe availability.`,
Suggestion: `Use single words like: "NONE", "TESTING", "PREVIEW", "GENERAL", "DEPRECATED", "SHUTDOWN"`,
}}

for _, tt := range []struct {
in string
expected []lint.Problem
}{
{"", nil},
{"x", nil},
{"x x", prob},
} {
api := &rpc.Api{
Availability: tt.in,
}
if availabilitySingleWord.OnlyIf(api) {
got := availabilitySingleWord.ApplyToApi(api)
if diff := cmp.Diff(got, tt.expected); diff != "" {
t.Errorf("unexpected diff: (-want +got):\n%s", diff)
}
}
}
}
66 changes: 66 additions & 0 deletions cmd/registry/cmd/check/rules/rule101/rule101.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2023 Google LLC. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Api recommended_version must be an ApiVersion that is a child of this Api.
package rule101

import (
"fmt"
"strings"

"github.com/apigee/registry/cmd/registry/cmd/check/lint"
"github.com/apigee/registry/rpc"
"github.com/apigee/registry/server/registry/names"
)

var ruleNum = 101
var ruleName = lint.NewRuleName(ruleNum, "api-recommended-version-ref")

// AddRules accepts a register function and registers each of
// this rules' checks to the RuleRegistry.
func AddRules(r lint.RuleRegistry) error {
return r.Register(
ruleNum,
recommendedVersionRef,
)
}

var recommendedVersionRef = &lint.ApiRule{
Name: ruleName,
OnlyIf: func(a *rpc.Api) bool {
return strings.TrimSpace(a.RecommendedVersion) != ""
},
ApplyToApi: func(a *rpc.Api) []lint.Problem {
versionName, err := names.ParseVersion(a.RecommendedVersion)
if err != nil {
return []lint.Problem{{
Severity: lint.ERROR,
Message: fmt.Sprintf(`recommended_version %q is not a valid ApiVersion name.`, a.RecommendedVersion),
Suggestion: fmt.Sprintf(`Parse error: %s`, err),
}}
}

apiName, _ := names.ParseApi(a.Name) // name assumed to be valid
if versionName.Api() != apiName {
return []lint.Problem{{
Severity: lint.ERROR,
Message: fmt.Sprintf(`recommended_version %q is not a child of this Api.`, a.RecommendedVersion),
Suggestion: `Correct the recommended_version.`,
}}
}

// TODO: check DB for existence
return nil
},
}
Loading

0 comments on commit fd1247b

Please sign in to comment.