registry apply --yaml for artifacts (#1174)

also improves test coverage

cmd/registry/cmd/apply/apply.go

@@ -29,6 +29,7 @@ func Command() *cobra.Command {
 	var project string
 	var recursive bool
 	var jobs int
+	var yamlArchives bool
 	cmd := &cobra.Command{
 		Use:   "apply (-f FILE | -f -)",
 		Short: "Apply YAML to the API Registry",
@@ -71,13 +72,17 @@ func Command() *cobra.Command {
 			if err := visitor.VerifyLocation(ctx, client, project); err != nil {
 				return fmt.Errorf("parent project %q does not exist: %s", project, err)
 			}
+			if yamlArchives { // TODO: remove when default
+				ctx = patch.SetStoreArchivesAsYaml(ctx)
+			}
 			return patch.Apply(ctx, client, adminClient, cmd.InOrStdin(), project, recursive, jobs, files...)
 		},
 	}
 	cmd.Flags().StringSliceVarP(&files, "file", "f", nil, "file or directory containing the patch(es) to apply. Use '-' to read from standard input")
 	_ = cmd.MarkFlagRequired("file")
 	cmd.Flags().StringVar(&project, "parent", "", "GCP project containing the API registry")
 	cmd.Flags().BoolVarP(&recursive, "recursive", "R", false, "process the directory used in -f, --file recursively")
+	cmd.Flags().BoolVarP(&yamlArchives, "yaml", "y", false, "store the archive data as yaml text instead of binary")
 	cmd.Flags().IntVarP(&jobs, "jobs", "j", 10, "number of actions to perform concurrently")
 	return cmd
 }

cmd/registry/cmd/apply/apply_test.go

@@ -19,10 +19,18 @@ import (
 	"os"
 	"testing"
 
+	"github.com/apigee/registry/pkg/application/apihub"
 	"github.com/apigee/registry/pkg/connection"
 	"github.com/apigee/registry/pkg/connection/grpctest"
+	"github.com/apigee/registry/pkg/encoding"
 	"github.com/apigee/registry/pkg/names"
+	"github.com/apigee/registry/pkg/visitor"
+	"github.com/apigee/registry/rpc"
 	"github.com/apigee/registry/server/registry"
+	"github.com/google/go-cmp/cmp"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/testing/protocmp"
+	"gopkg.in/yaml.v3"
 )
 
 // TestMain will set up a local RegistryServer and grpc.Server for all
@@ -158,3 +166,124 @@ func TestApply_Stdin(t *testing.T) {
 		})
 	}
 }
+
+func TestArtifactStorage(t *testing.T) {
+	project := names.Project{ProjectID: "apply-test"}
+	parent := project.String() + "/locations/global"
+	artifactName, _ := names.ParseArtifact(parent + "/artifacts/lifecycle")
+	file := sampleDir + "/artifacts/lifecycle.yaml"
+
+	ctx := context.Background()
+	client, _ := grpctest.SetupRegistry(ctx, t, project.ProjectID, nil)
+
+	// store as yaml
+	t.Run("store as yaml", func(t *testing.T) {
+		cmd := Command()
+		args := []string{"-f", file, "--parent", parent, "--jobs", "1", "--yaml"}
+		cmd.SetArgs(args)
+		if err := cmd.Execute(); err != nil {
+			t.Fatalf("Execute() with args %+v returned error: %s", args, err)
+		}
+
+		if err := visitor.GetArtifact(ctx, client, artifactName, true, func(ctx context.Context, message *rpc.Artifact) error {
+			var encArt encoding.Artifact
+			artBytes, err := os.ReadFile(file)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err := yaml.Unmarshal(artBytes, &encArt); err != nil {
+				t.Fatal(err)
+			}
+
+			artYamlBytes, err := yaml.Marshal(encArt.Data)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if diff := cmp.Diff(artYamlBytes, message.GetContents(), nil); diff != "" {
+				t.Errorf("unexpected diff (-want +got):\n%s", diff)
+			}
+			return nil
+		}); err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	t.Run("store as proto", func(t *testing.T) {
+		cmd := Command()
+		args := []string{"-f", file, "--parent", parent, "--jobs", "1"}
+		cmd.SetArgs(args)
+		if err := cmd.Execute(); err != nil {
+			t.Fatalf("Execute() with args %+v returned error: %s", args, err)
+		}
+
+		ac, err := client.GetArtifactContents(ctx, &rpc.GetArtifactContentsRequest{
+			Name: artifactName.String(),
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		lifecycle := new(apihub.Lifecycle)
+		if err = proto.Unmarshal(ac.Data, lifecycle); err != nil {
+			t.Fatal(err)
+		}
+
+		want := &apihub.Lifecycle{
+			Id:          "lifecycle", // deprecated field
+			Kind:        "Lifecycle", // deprecated field
+			DisplayName: "Lifecycle",
+			Description: "A series of stages that an API typically moves through in its lifetime",
+			Stages: []*apihub.Lifecycle_Stage{
+				{
+					Id:           "concept",
+					DisplayName:  "Concept",
+					Description:  "Description of the business case and user needs for why an API should exist",
+					DisplayOrder: 0,
+				},
+				{
+					Id:           "design",
+					DisplayName:  "Design",
+					Description:  "Definition of the interface details and proposal of the API contract",
+					DisplayOrder: 1,
+				},
+				{
+					Id:           "develop",
+					DisplayName:  "Develop",
+					Description:  "Implementation of the service and its API",
+					DisplayOrder: 2,
+				},
+				{
+					Id:           "preview",
+					DisplayName:  "Preview",
+					Description:  "Staging of implementations in the pre-production phase",
+					DisplayOrder: 3,
+				},
+				{
+					Id:           "production",
+					DisplayName:  "Production",
+					Description:  "API available for production workloads",
+					DisplayOrder: 4,
+				},
+				{
+					Id:           "deprecated",
+					DisplayName:  "Deprecated",
+					Description:  "API not recommended for new consumers",
+					DisplayOrder: 5,
+				},
+				{
+					Id:           "retired",
+					DisplayName:  "Retired",
+					Description:  "API no longer available for use",
+					DisplayOrder: 6,
+				},
+			},
+		}
+
+		opts := cmp.Options{
+			protocmp.Transform(),
+		}
+		if !cmp.Equal(want, lifecycle, opts) {
+			t.Errorf("unexpected diff (-want +got):\n%s", cmp.Diff(want, lifecycle, opts))
+		}
+	})
+}

cmd/registry/patch/artifact.go

@@ -103,6 +103,17 @@ func artifactName(parent string, metadata encoding.Metadata) (names.Artifact, er
 	return names.ParseArtifact(parent + "/artifacts/" + metadata.Name)
 }
 
+// TODO: remove when default
+var yamlArchiveKey = struct{}{}
+
+func SetStoreArchivesAsYaml(ctx context.Context) context.Context {
+	return context.WithValue(ctx, yamlArchiveKey, true)
+}
+
+func storeArchivesAsYaml(ctx context.Context) bool {
+	return ctx.Value(yamlArchiveKey) != nil && ctx.Value(yamlArchiveKey).(bool)
+}
+
 func applyArtifactPatch(ctx context.Context, client connection.RegistryClient, content *encoding.Artifact, parent string, filename string) error {
 	// Restyle the YAML representation so that yaml.Marshal will marshal it as JSON.
 	encoding.StyleForJSON(&content.Data)
@@ -116,6 +127,7 @@ func applyArtifactPatch(ctx context.Context, client connection.RegistryClient, c
 	if err != nil {
 		return err
 	}
+	var mimeType string
 	var bytes []byte
 	// Unmarshal the JSON serialization into the message struct.
 	var m proto.Message
@@ -131,26 +143,38 @@ func applyArtifactPatch(ctx context.Context, client connection.RegistryClient, c
 				}
 			}
 		}
-		// Marshal the message struct to bytes.
-		bytes, err = proto.Marshal(m)
-		if err != nil {
-			return err
+		if storeArchivesAsYaml(ctx) {
+			mimeType = mime.YamlMimeTypeForKind(content.Kind)
+			encoding.StyleForYAML(&content.Data)
+			bytes, err = yaml.Marshal(content.Data)
+			if err != nil {
+				return err
+			}
+		} else {
+			mimeType = mime.MimeTypeForKind(content.Kind)
+			// Marshal the message struct to bytes.
+			bytes, err = proto.Marshal(m)
+			if err != nil {
+				return err
+			}
 		}
 	} else {
 		// If there was no struct defined for the type, marshal it struct as YAML
+		mimeType = mime.MimeTypeForKind(content.Kind)
 		encoding.StyleForYAML(&content.Data)
 		bytes, err = yaml.Marshal(content.Data)
 		if err != nil {
 			return err
 		}
 	}
+
 	name, err := artifactName(parent, content.Header.Metadata)
 	if err != nil {
 		return err
 	}
 	artifact := &rpc.Artifact{
 		Name:        name.String(),
-		MimeType:    mime.MimeTypeForKind(content.Kind),
+		MimeType:    mimeType,
 		Contents:    bytes,
 		Labels:      content.Metadata.Labels,
 		Annotations: content.Metadata.Annotations,

pkg/mime/types.go

@@ -93,6 +93,12 @@ func MimeTypeForMessageType(protoType string) string {
 
 // MessageTypeForMimeType returns the Protocol Buffer message type represented by a MIME type.
 func MessageTypeForMimeType(protoType string) (string, error) {
+	re1 := regexp.MustCompile("^application/yaml;type=(.*)$")
+	m1 := re1.FindStringSubmatch(protoType)
+	if m1 != nil && len(m1[1]) > 0 {
+		return strings.TrimSuffix(m1[1], "+gzip"), nil
+	}
+
 	re := regexp.MustCompile("^application/octet-stream;type=(.*)$")
 	m := re.FindStringSubmatch(protoType)
 	if m == nil || len(m) < 2 || len(m[1]) == 0 {
@@ -104,7 +110,9 @@ func MessageTypeForMimeType(protoType string) (string, error) {
 // KindForMimeType returns the name to be used as the "kind" of an exported artifact.
 func KindForMimeType(mimeType string) string {
 	if strings.HasPrefix(mimeType, "application/yaml;type=") {
-		return strings.TrimPrefix(mimeType, "application/yaml;type=")
+		typeParameter := strings.TrimPrefix(mimeType, "application/yaml;type=")
+		parts := strings.Split(typeParameter, ".")
+		return parts[len(parts)-1]
 	} else if strings.HasPrefix(mimeType, "application/octet-stream;type=") {
 		typeParameter := strings.TrimPrefix(mimeType, "application/octet-stream;type=")
 		parts := strings.Split(typeParameter, ".")
@@ -150,6 +158,19 @@ func MimeTypeForKind(kind string) string {
 	return fmt.Sprintf("application/yaml;type=%s", kind)
 }
 
+// YamlMimeTypeForKind returns a YAML mime type that corresponds to a kind.
+func YamlMimeTypeForKind(kind string) string {
+	if kind == "" {
+		return "application/yaml"
+	}
+	for k := range artifactMessageTypes {
+		if strings.HasSuffix(k, "."+kind) {
+			return fmt.Sprintf("application/yaml;type=%s", k)
+		}
+	}
+	return fmt.Sprintf("application/yaml;type=%s", kind)
+}
+
 // messageFactory represents functions that construct message structs.
 type messageFactory func() proto.Message
 

pkg/mime/types_test.go

@@ -254,6 +254,16 @@ func TestProtobufMessageTypes(t *testing.T) {
 			messageType: "gnostic.metrics.Vocabulary",
 			mimeType:    "application/octet-stream;type=gnostic.metrics.Vocabulary",
 		},
+		{
+			kind:        "FieldSet",
+			messageType: "google.cloud.apigeeregistry.v1.apihub.FieldSet",
+			mimeType:    "application/octet-stream;type=google.cloud.apigeeregistry.v1.apihub.FieldSet",
+		},
+		{
+			kind:        "FieldSetDefinition",
+			messageType: "google.cloud.apigeeregistry.v1.apihub.FieldSetDefinition",
+			mimeType:    "application/octet-stream;type=google.cloud.apigeeregistry.v1.apihub.FieldSetDefinition",
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.kind, func(t *testing.T) {
@@ -293,6 +303,24 @@ func TestProtobufMessageTypes(t *testing.T) {
 			if type2 != test.kind {
 				t.Errorf("incorrect message for kind, expected %s got %s", test.kind, type2)
 			}
+
+			// yaml variations of the above
+			yamlType := strings.Replace(test.mimeType, "octet-stream", "yaml", 1)
+			value = YamlMimeTypeForKind(test.kind)
+			if value != yamlType {
+				t.Errorf("incorrect mime type for kind, expected %s got %s", yamlType, value)
+			}
+			value = KindForMimeType(yamlType)
+			if value != test.kind {
+				t.Errorf("incorrect kind, expected %s got %s", test.kind, value)
+			}
+			value, err = MessageTypeForMimeType(yamlType)
+			if err != nil {
+				t.Errorf("Error getting message type for mime type %s", err)
+			}
+			if value != test.messageType {
+				t.Errorf("incorrect message type, expected %s got %s", test.messageType, value)
+			}
 		})
 	}
 }
@@ -317,6 +345,10 @@ func TestYamlMessageTypes(t *testing.T) {
 			if value != test.mimeType {
 				t.Errorf("incorrect mime type for kind, expected %s got %s", test.mimeType, value)
 			}
+			value = YamlMimeTypeForKind(test.kind)
+			if value != test.mimeType {
+				t.Errorf("incorrect mime type for kind, expected %s got %s", test.mimeType, value)
+			}
 			value = KindForMimeType(test.mimeType)
 			if value != test.kind {
 				t.Errorf("incorrect kind, expected %s got %s", test.kind, value)