Merge pull request GoogleCloudPlatform#2299 from justinsb/gapic_add_c…

…omments

mockgcp: add comments to protos generated from OpenAPI

mockgcp/Makefile

@@ -121,18 +121,18 @@ generate-protos-from-openapi:
 
 	wget -O temp/bigquery-v2.json https://raw.githubusercontent.com/googleapis/google-api-go-client/main/bigquery/v2/bigquery-api.json
 	mkdir -p apis/mockgcp/cloud/bigquery/v2/
-	cd tools/gapic; go run . --proto-version=2 ../../temp/bigquery-v2.json > ../../apis/mockgcp/cloud/bigquery/v2/api.proto
+	cd tools/gapic; go run . --proto-version=2 --proto-package mockgcp.cloud.bigquery.v2 ../../temp/bigquery-v2.json > ../../apis/mockgcp/cloud/bigquery/v2/api.proto
 
 	wget -O temp/servicenetworking-api.json https://raw.githubusercontent.com/googleapis/google-api-go-client/main/servicenetworking/v1/servicenetworking-api.json
 	mkdir -p apis/mockgcp/cloud/servicenetworking/v1/
-	cd tools/gapic; go run . ../../temp/servicenetworking-api.json > ../../apis/mockgcp/cloud/servicenetworking/v1/servicenetworking.proto
+	cd tools/gapic; go run . --proto-package mockgcp.cloud.servicenetworking.v1 ../../temp/servicenetworking-api.json > ../../apis/mockgcp/cloud/servicenetworking/v1/servicenetworking.proto
 
 	wget -O temp/storage-v1.json https://raw.githubusercontent.com/googleapis/google-api-go-client/main/storage/v1/storage-api.json
 	mkdir -p apis/mockgcp/storage/v1/
-	cd tools/gapic; go run . --proto-version=2 ../../temp/storage-v1.json > ../../apis/mockgcp/storage/v1/service.proto
+	cd tools/gapic; go run . --proto-version=2 --proto-package mockgcp.storage.v1 ../../temp/storage-v1.json > ../../apis/mockgcp/storage/v1/service.proto
 
 	wget -O temp/ids-api-v1.json https://raw.githubusercontent.com/googleapis/google-api-go-client/b49e3b908a8ed562e068736f1c42e992538ba6e0/ids/v1/ids-api.json
 	mkdir -p apis/mockgcp/cloud/ids/v1/
-	cd tools/gapic; go run . --proto-version=2 ../../temp/ids-api-v1.json > ../../apis/mockgcp/cloud/ids/v1/service.proto
+	cd tools/gapic; go run . --proto-version=2 --proto-package mockgcp.cloud.ids.v1 ../../temp/ids-api-v1.json > ../../apis/mockgcp/cloud/ids/v1/service.proto
 
 	rm -r temp

mockgcp/tools/gapic/main.go

@@ -44,8 +44,14 @@ func run(ctx context.Context) error {
 
 	protoVersion := 3
 	flag.IntVar(&protoVersion, "proto-version", protoVersion, "use proto version (2 or 3)")
+	protoPackage := ""
+	flag.StringVar(&protoPackage, "proto-package", protoPackage, "protobuf package to generate")
 	flag.Parse()
 
+	if protoPackage == "" {
+		return fmt.Errorf("must specify --proto-package")
+	}
+
 	p := flag.Args()[0]
 	b, err := os.ReadFile(p)
 	if err != nil {
@@ -58,7 +64,7 @@ func run(ctx context.Context) error {
 		return fmt.Errorf("parsing json %q: %w", p, err)
 	}
 
-	c := protogen.NewOpenAPIConverter(doc)
+	c := protogen.NewOpenAPIConverter(protoPackage, doc)
 	fileDescriptor, err := c.Convert(ctx)
 	if err != nil {
 		return fmt.Errorf("convert failed: %w", err)
@@ -82,6 +88,7 @@ func run(ctx context.Context) error {
 
 	files.RangeFiles(func(file protoreflect.FileDescriptor) bool {
 		pw := protogen.NewProtoWriter(os.Stdout)
+		pw.SetComments(&c.Comments)
 		pw.SetProtoVersion(protoVersion)
 		pw.WriteFile(file)
 		if err := pw.Error(); err != nil {

mockgcp/tools/gapic/pkg/protogen/convert.go

@@ -37,12 +37,44 @@ type OpenAPIConverter struct {
 	imports map[string]bool
 
 	fileDescriptor *descriptorpb.FileDescriptorProto
+
+	// Comments holds field and message level comments
+	Comments
+
+	// protoPackageName is the name of the proto package we are generating.
+	protoPackageName string
+}
+
+// Comments holds all the comments for our messages / fields
+type Comments struct {
+	comments map[string]*Comment
+}
+
+func (c *Comments) SetComment(key string, comment string) {
+	c.comments[key] = &Comment{Text: comment}
+
+}
+func (c *Comments) GetComment(key string) string {
+	o, found := c.comments[key]
+	if !found {
+		return ""
+	}
+	return o.Text
 }
 
-func NewOpenAPIConverter(doc *openapi.Document) *OpenAPIConverter {
+// Comment is a comment on a message/field
+type Comment struct {
+	Text string
+}
+
+func NewOpenAPIConverter(protoPackageName string, doc *openapi.Document) *OpenAPIConverter {
 	return &OpenAPIConverter{
-		doc:     doc,
-		imports: make(map[string]bool),
+		doc:              doc,
+		protoPackageName: protoPackageName,
+		imports:          make(map[string]bool),
+		Comments: Comments{
+			comments: make(map[string]*Comment),
+		},
 	}
 }
 
@@ -54,6 +86,7 @@ func (c *OpenAPIConverter) buildMessageFromOpenAPI(message *openapi.Property) (*
 	nextTag := int32(1)
 	desc := &descriptorpb.DescriptorProto{}
 	desc.Name = PtrTo(message.ID)
+	c.SetComment(c.protoPackageName+"."+desc.GetName(), message.Description)
 	for _, entry := range message.Properties.Entries() {
 		propertyID := entry.Key
 		property := entry.Value
@@ -212,6 +245,7 @@ func (c *OpenAPIConverter) buildMessageFromOpenAPI(message *openapi.Property) (*
 			klog.Fatalf("unsupported property.Type %q %+v", property.Type, property)
 		}
 
+		c.SetComment(c.protoPackageName+"."+desc.GetName()+"."+field.GetName(), property.Description)
 		desc.Field = append(desc.Field, field)
 	}
 
@@ -543,7 +577,7 @@ func (c *OpenAPIConverter) buildServiceFromOpenAPI(pluralName string, resource *
 
 		service.Method = append(service.Method, serviceMethod)
 
-		klog.Infof("%s/%s => %v", singularName, methodName, prototext.Format(serviceMethod))
+		klog.V(4).Infof("%s/%s => %v", singularName, methodName, prototext.Format(serviceMethod))
 	}
 
 	c.fileDescriptor.Service = append(c.fileDescriptor.Service, service)
@@ -553,12 +587,14 @@ func (c *OpenAPIConverter) buildServiceFromOpenAPI(pluralName string, resource *
 func (c *OpenAPIConverter) Convert(ctx context.Context) (*descriptorpb.FileDescriptorProto, error) {
 
 	{
-		version := c.doc.Version
-		serviceID := c.doc.Name
-		prefix := []string{"google", "cloud"}
-
-		name := path.Join(path.Join(prefix...), serviceID, version, serviceID+"pb.proto")
-		packageName := strings.Join(prefix, ".") + "." + serviceID + "." + version
+		tokens := strings.Split(c.protoPackageName, ".")
+		if len(tokens) < 2 {
+			return nil, fmt.Errorf("unexpected proto package name %q (expected more tokens)", c.protoPackageName)
+		}
+		name := path.Join(tokens...) + ".pb.proto"
+		packageName := c.protoPackageName
+		serviceID := tokens[len(tokens)-2]
+		version := tokens[len(tokens)-1]
 		goPackageName := path.Join("cloud.google.com/go", serviceID, "api"+version, serviceID+"pb") + ";" + serviceID + "pb"
 		c.fileDescriptor = &descriptorpb.FileDescriptorProto{
 			Name:    PtrTo(name),
@@ -579,7 +615,7 @@ func (c *OpenAPIConverter) Convert(ctx context.Context) (*descriptorpb.FileDescr
 			}
 			c.fileDescriptor.MessageType = append(c.fileDescriptor.MessageType, desc)
 
-			klog.Infof("%s => %+v\n", schemaName, prototext.Format(desc))
+			klog.V(4).Infof("%s => %+v\n", schemaName, prototext.Format(desc))
 		} else if message.Type == "any" {
 			klog.Warningf("skipping schema with type any: %q", message.ID)
 		} else {

mockgcp/tools/gapic/pkg/protogen/render.go

@@ -33,6 +33,7 @@ type ProtoWriter struct {
 	w            io.Writer
 	errors       []error
 	protoVersion int
+	comments     *Comments
 }
 
 func NewProtoWriter(w io.Writer) *ProtoWriter {
@@ -46,6 +47,17 @@ func (p *ProtoWriter) SetProtoVersion(protoVersion int) {
 	p.protoVersion = protoVersion
 }
 
+func (p *ProtoWriter) SetComments(comments *Comments) {
+	p.comments = comments
+}
+
+func (p *ProtoWriter) getComment(obj protoreflect.FullName) string {
+	if p.comments == nil {
+		return ""
+	}
+	return p.comments.GetComment(string(obj))
+}
+
 func (p *ProtoWriter) Error() error {
 	if len(p.errors) > 0 {
 		return errors.Join(p.errors...)
@@ -82,6 +94,13 @@ func (p *ProtoWriter) nameForMessageType(md protoreflect.MessageDescriptor) stri
 }
 
 func (p *ProtoWriter) renderField(fd protoreflect.FieldDescriptor) {
+	comment := p.getComment(fd.FullName())
+	if comment != "" {
+		for _, line := range strings.Split(comment, "\n") {
+			p.printf("  // %s\n", line)
+		}
+	}
+
 	var b bytes.Buffer
 	b.WriteString("  ")
 
@@ -165,6 +184,13 @@ func (p *ProtoWriter) renderField(fd protoreflect.FieldDescriptor) {
 }
 
 func (p *ProtoWriter) renderMessage(msg protoreflect.MessageDescriptor) {
+	comment := p.getComment(msg.FullName())
+	if comment != "" {
+		for _, line := range strings.Split(comment, "\n") {
+			p.printf("  // %s\n", line)
+		}
+	}
+
 	p.printf("message %s {\n", msg.Name())
 	fields := msg.Fields()
 	for i := 0; i < fields.Len(); i++ {