Skip to content

Commit

Permalink
When registry get matches nothing, print "Not Found" to stderr and …
Browse files Browse the repository at this point in the history
…don't report an error. (#1073)
  • Loading branch information
timburks authored Mar 8, 2023
1 parent 4ee8fd6 commit c46b9de
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 9 deletions.
16 changes: 14 additions & 2 deletions cmd/registry/cmd/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"github.com/apigee/registry/pkg/visitor"
"github.com/apigee/registry/rpc"
"github.com/spf13/cobra"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
)
Expand Down Expand Up @@ -75,9 +77,19 @@ func Command() *cobra.Command {
Pattern: pattern,
Filter: filter,
}); err != nil {
if status.Code(err) == codes.NotFound {
fmt.Fprintln(cmd.ErrOrStderr(), "Not Found")
return nil
}
return err
}
return v.write()
// Write any accumulated output.
err = v.write()
if err != nil && status.Code(err) == codes.NotFound {
fmt.Fprintln(cmd.ErrOrStderr(), "Not Found")
return nil
}
return err
},
}

Expand Down Expand Up @@ -273,7 +285,7 @@ func newOutputTypeError(resourceType, outputType string) error {

func (v *getVisitor) write() error {
if len(v.results) == 0 {
return fmt.Errorf("no matching results found")
return status.Error(codes.NotFound, "no matching results found")
}
if v.output == "yaml" {
var result interface{}
Expand Down
53 changes: 46 additions & 7 deletions cmd/registry/cmd/get/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,27 @@ func TestGetValidResources(t *testing.T) {
}
})
}
patternsThatDontMatchAnything := []string{
"projects/my-project/locations/global/apis/-/artifacts/xx",
"projects/my-project/locations/global/apis/-/versions/-/artifacts/xx",
"projects/my-project/locations/global/apis/-/versions/-/specs/-/artifacts/xx",
"projects/my-project/locations/global/apis/-/deployments/-/artifacts/xx",
}
for _, r := range patternsThatDontMatchAnything {
t.Run(r+"+nothing", func(t *testing.T) {
cmd := Command()
args := []string{r, "-o", "name"}
cmd.SetArgs(args)
out := bytes.NewBuffer(make([]byte, 0))
cmd.SetOut(out)
if err := cmd.Execute(); err != nil {
t.Errorf("Execute() with args %v returned error: %s", args, err)
}
if len(out.Bytes()) != 0 {
t.Errorf("Execute() with args %v failed to return expected value(s)", args)
}
})
}
}

func TestGetInvalidResources(t *testing.T) {
Expand Down Expand Up @@ -258,14 +279,27 @@ func TestGetInvalidResources(t *testing.T) {
invalid := []string{
"projects/my-project/locations/global/invalid",
"projects/my-project/locations/global/apis/-/invalid",
"projects/my-project/locations/global/apis/-/artifacts/xx",
"projects/my-project/locations/global/apis/-/versions/-/artifacts/xx",
"projects/my-project/locations/global/apis/-/versions/-/specs/-/artifacts/xx",
"projects/my-project/locations/global/apis/-/deployments/-/artifacts/xx",
"projects/my-project/locations/global/apis/a/invalid",
"projects/my-project/locations/global/apis/a/versions/v/invalid",
"projects/my-project/locations/global/apis/a/versions/v/specs/s/invalid",
"projects/my-project/locations/global/apis/a/deployments/d/invalid",
}
for i, r := range invalid {
t.Run(r, func(t *testing.T) {
// cycle through output types
format := []string{"name", "yaml", "contents"}[i%3]
cmd := Command()
cmd.SilenceUsage = true
cmd.SilenceErrors = true
args := []string{r, "-o", format}
cmd.SetArgs(args)
if err := cmd.Execute(); err == nil {
t.Errorf("Execute() with args %v succeeded but should have failed", args)
}
})
}
// Verify that "not found" is not treated an error.
notfound := []string{
"projects/my-project/locations/global/artifacts/xx",
"projects/my-project/locations/global/apis/a/versions/vv",
"projects/my-project/locations/global/apis/a/versions/v/specs/ss",
Expand All @@ -279,17 +313,22 @@ func TestGetInvalidResources(t *testing.T) {
"projects/my-project/locations/global/apis/a/versions/v/specs/s/artifacts/xx",
"projects/my-project/locations/global/apis/a/deployments/d/artifacts/xx",
}
for i, r := range invalid {
for i, r := range notfound {
t.Run(r, func(t *testing.T) {
// cycle through output types
format := []string{"name", "yaml", "contents"}[i%3]
cmd := Command()
cmd.SilenceUsage = true
cmd.SilenceErrors = true
out := bytes.NewBuffer(make([]byte, 0))
cmd.SetErr(out)
args := []string{r, "-o", format}
cmd.SetArgs(args)
if err := cmd.Execute(); err == nil {
t.Errorf("Execute() with args %v succeeded but should have failed", args)
if err := cmd.Execute(); err != nil {
t.Errorf("Execute() with args %v failed %s", args, err)
}
if out.String() != "Not Found\n" {
t.Errorf("Execute() with args %v failed to return expected value(s)", args)
}
})
}
Expand Down

0 comments on commit c46b9de

Please sign in to comment.