Repository: apigee/registry (go)

Issue:
Registry Tool: bulk upload commands don't validate project ID flag
Running the bulk upload commands without specifying a project ID will proceed without error until it attempts to use the project ID in a request to the server. Failing to specify the project ID should immediately fail with an informative error message.

```
$ registry upload bulk openapi ~/Desktop/openapi-directory
  INFO[0000] Uploading apis/apis-1forge.com/versions/0.0.1/specs/swagger.yaml uid=7aa38e1e
 DEBUG[0000] Failed to create API projects//locations/global/apis/apis-1forge.com error=rpc error: code = InvalidArgument desc = invalid API name "projects//locations/global/apis/apis-1forge.com": must match "^projects/([A-Za-z0-9-.]+)/locations/global/apis/([A-Za-z0-9-.]+)$" uid=7aa38e1e
```

```
$ registry upload bulk protos ~/Desktop/googleapis/                                        
  INFO[0000] Uploading apis/actions/versions/v2/specs/google-actions-sdk-v2.zip uid=7c0971c6
 DEBUG[0000] Failed to create API projects//locations/global/apis/actions error=rpc error: code = InvalidArgument desc = invalid API name "projects//locations/global/apis/actions": must match "^projects/([A-Za-z0-9-.]+)/locations/global/apis/([A-Za-z0-9-.]+)$" uid=7c0971c6
```

Pull request:
Verify that bulk upload parent exists, improve command tests.
Fixes #472.

Testing this required some changes discussed in #827.

Patch:
diff --git a/cmd/registry/cmd/upload/bulk/discovery.go b/cmd/registry/cmd/upload/bulk/discovery.go
index 1a8768d..78b20b8 100644
--- a/cmd/registry/cmd/upload/bulk/discovery.go
+++ b/cmd/registry/cmd/upload/bulk/discovery.go
@@ -35,17 +35,19 @@ func discoveryCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "discovery",
 		Short: "Bulk-upload API Discovery documents from the Google API Discovery service",
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
 			parent, err := getParent(cmd)
 			if err != nil {
-				log.FromContext(ctx).WithError(err).Fatal("Failed to identify parent project")
+				return fmt.Errorf("failed to identify parent project (%s)", err)
 			}
 			client, err := connection.NewRegistryClient(ctx)
 			if err != nil {
 				log.FromContext(ctx).WithError(err).Fatal("Failed to get client")
 			}
-
+			if err := core.VerifyLocation(ctx, client, parent); err != nil {
+				return fmt.Errorf("parent does not exist (%s)", err)
+			}
 			// create a queue for upload tasks and wait for the workers to finish after filling it.
 			jobs, err := cmd.Flags().GetInt("jobs")
 			if err != nil {
@@ -70,6 +72,7 @@ func discoveryCommand() *cobra.Command {
 					specID:    "discovery.json",
 				}
 			}
+			return nil
 		},
 	}
 
diff --git a/cmd/registry/cmd/upload/bulk/openapi.go b/cmd/registry/cmd/upload/bulk/openapi.go
index aee8cdd..bf29674 100644
--- a/cmd/registry/cmd/upload/bulk/openapi.go
+++ b/cmd/registry/cmd/upload/bulk/openapi.go
@@ -39,17 +39,19 @@ func openAPICommand() *cobra.Command {
 		Use:   "openapi",
 		Short: "Bulk-upload OpenAPI descriptions from a directory of specs",
 		Args:  cobra.MinimumNArgs(1),
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
 			parent, err := getParent(cmd)
 			if err != nil {
-				log.FromContext(ctx).WithError(err).Fatal("Failed to identify parent project")
+				return fmt.Errorf("failed to identify parent project (%s)", err)
 			}
 			client, err := connection.NewRegistryClient(ctx)
 			if err != nil {
 				log.FromContext(ctx).WithError(err).Fatal("Failed to get client")
 			}
-
+			if err := core.VerifyLocation(ctx, client, parent); err != nil {
+				return fmt.Errorf("parent does not exist (%s)", err)
+			}
 			// create a queue for upload tasks and wait for the workers to finish after filling it.
 			jobs, err := cmd.Flags().GetInt("jobs")
 			if err != nil {
@@ -65,6 +67,7 @@ func openAPICommand() *cobra.Command {
 				}
 				scanDirectoryForOpenAPI(ctx, client, parent, baseURI, path, taskQueue)
 			}
+			return nil
 		},
 	}
 
diff --git a/cmd/registry/cmd/upload/bulk/protos.go b/cmd/registry/cmd/upload/bulk/protos.go
index 8708b32..d9d0250 100644
--- a/cmd/registry/cmd/upload/bulk/protos.go
+++ b/cmd/registry/cmd/upload/bulk/protos.go
@@ -58,17 +58,19 @@ func protosCommand() *cobra.Command {
 		Use:   "protos PATH",
 		Short: "Bulk-upload Protocol Buffer descriptions from a directory of specs",
 		Args:  cobra.ExactArgs(1),
-		Run: func(cmd *cobra.Command, args []string) {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
 			parent, err := getParent(cmd)
 			if err != nil {
-				log.FromContext(ctx).WithError(err).Fatal("Failed to identify parent project")
+				return fmt.Errorf("failed to identify parent project (%s)", err)
 			}
 			client, err := connection.NewRegistryClient(ctx)
 			if err != nil {
 				log.FromContext(ctx).WithError(err).Fatal("Failed to get client")
 			}
-
+			if err := core.VerifyLocation(ctx, client, parent); err != nil {
+				return fmt.Errorf("parent does not exist (%s)", err)
+			}
 			// create a queue for upload tasks and wait for the workers to finish after filling it.
 			jobs, err := cmd.Flags().GetInt("jobs")
 			if err != nil {
@@ -95,6 +97,7 @@ func protosCommand() *cobra.Command {
 					log.FromContext(ctx).WithError(err).Debug("Failed to walk directory")
 				}
 			}
+			return nil
 		},
 	}
 
diff --git a/cmd/registry/core/verify.go b/cmd/registry/core/verify.go
new file mode 100644
index 0000000..10677bb
--- /dev/null
+++ b/cmd/registry/core/verify.go
@@ -0,0 +1,16 @@
+package core
+
+import (
+	"context"
+
+	"github.com/apigee/registry/pkg/connection"
+	"github.com/apigee/registry/rpc"
+)
+
+func VerifyLocation(ctx context.Context, client connection.RegistryClient, location string) error {
+	_, err := client.GrpcClient().ListApis(ctx, &rpc.ListApisRequest{
+		Parent:   location,
+		PageSize: 1,
+	})
+	return err
+}


Test patch:
diff --git a/cmd/registry/cmd/upload/bulk/discovery_test.go b/cmd/registry/cmd/upload/bulk/discovery_test.go
new file mode 100644
index 0000000..be82750
--- /dev/null
+++ b/cmd/registry/cmd/upload/bulk/discovery_test.go
@@ -0,0 +1,55 @@
+// Copyright 2022 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 bulk
+
+import (
+	"testing"
+)
+
+func TestDiscoveryMissingParent(t *testing.T) {
+	const (
+		projectID   = "missing"
+		projectName = "projects/" + projectID
+		parent      = projectName + "/locations/global"
+	)
+	tests := []struct {
+		desc string
+		args []string
+	}{
+		{
+			desc: "parent",
+			args: []string{"discovery", "--parent", parent},
+		},
+		{
+			desc: "project-id",
+			args: []string{"discovery", "--project-id", projectID},
+		},
+		{
+			desc: "unspecified",
+			args: []string{"discovery"},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			cmd := Command()
+			cmd.SilenceUsage = true
+			cmd.SilenceErrors = true
+			cmd.SetArgs(test.args)
+			if cmd.Execute() == nil {
+				t.Error("expected error, none reported")
+			}
+		})
+	}
+}
diff --git a/cmd/registry/cmd/upload/bulk/openapi_test.go b/cmd/registry/cmd/upload/bulk/openapi_test.go
index 724ece7..d1364c1 100644
--- a/cmd/registry/cmd/upload/bulk/openapi_test.go
+++ b/cmd/registry/cmd/upload/bulk/openapi_test.go
@@ -111,3 +111,39 @@ func TestOpenAPI(t *testing.T) {
 		t.Fatalf("Failed to delete test project: %s", err)
 	}
 }
+
+func TestOpenAPIMissingParent(t *testing.T) {
+	const (
+		projectID   = "missing"
+		projectName = "projects/" + projectID
+		parent      = projectName + "/locations/global"
+	)
+	tests := []struct {
+		desc string
+		args []string
+	}{
+		{
+			desc: "parent",
+			args: []string{"openapi", "nonexistent-specs-dir", "--parent", parent},
+		},
+		{
+			desc: "project-id",
+			args: []string{"openapi", "nonexistent-specs-dir", "--project-id", projectID},
+		},
+		{
+			desc: "unspecified",
+			args: []string{"openapi", "nonexistent-specs-dir"},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			cmd := Command()
+			cmd.SilenceUsage = true
+			cmd.SilenceErrors = true
+			cmd.SetArgs(test.args)
+			if cmd.Execute() == nil {
+				t.Error("expected error, none reported")
+			}
+		})
+	}
+}
diff --git a/cmd/registry/cmd/upload/bulk/protos_test.go b/cmd/registry/cmd/upload/bulk/protos_test.go
index 2589286..296139d 100644
--- a/cmd/registry/cmd/upload/bulk/protos_test.go
+++ b/cmd/registry/cmd/upload/bulk/protos_test.go
@@ -145,3 +145,39 @@ func TestProtos(t *testing.T) {
 		t.Fatalf("Failed to delete test project: %s", err)
 	}
 }
+
+func TestProtosMissingParent(t *testing.T) {
+	const (
+		projectID   = "missing"
+		projectName = "projects/" + projectID
+		parent      = projectName + "/locations/global"
+	)
+	tests := []struct {
+		desc string
+		args []string
+	}{
+		{
+			desc: "parent",
+			args: []string{"protos", "nonexistent-specs-dir", "--parent", parent},
+		},
+		{
+			desc: "project-id",
+			args: []string{"protos", "nonexistent-specs-dir", "--project-id", projectID},
+		},
+		{
+			desc: "unspecified",
+			args: []string{"protos", "nonexistent-specs-dir"},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			cmd := Command()
+			cmd.SilenceUsage = true
+			cmd.SilenceErrors = true
+			cmd.SetArgs(test.args)
+			if cmd.Execute() == nil {
+				t.Error("expected error, none reported")
+			}
+		})
+	}
+}