From c46b9def58e3999b7fb04a7679d968af51a3a8db Mon Sep 17 00:00:00 2001
From: Tim Burks <timburks@google.com>
Date: Wed, 8 Mar 2023 13:04:06 -0800
Subject: [PATCH] When `registry get` matches nothing, print "Not Found" to
 stderr and don't report an error. (#1073)

---
 cmd/registry/cmd/get/get.go      | 16 ++++++++--
 cmd/registry/cmd/get/get_test.go | 53 +++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/cmd/registry/cmd/get/get.go b/cmd/registry/cmd/get/get.go
index 600b2780d..0609b040b 100644
--- a/cmd/registry/cmd/get/get.go
+++ b/cmd/registry/cmd/get/get.go
@@ -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"
 )
@@ -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
 		},
 	}
 
@@ -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{}
diff --git a/cmd/registry/cmd/get/get_test.go b/cmd/registry/cmd/get/get_test.go
index 130542d85..bf5e57d53 100644
--- a/cmd/registry/cmd/get/get_test.go
+++ b/cmd/registry/cmd/get/get_test.go
@@ -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) {
@@ -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",
@@ -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)
 			}
 		})
 	}