Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mockgcp: add comments to protos generated from OpenAPI #2299

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions mockgcp/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,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
9 changes: 8 additions & 1 deletion mockgcp/tools/gapic/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
58 changes: 47 additions & 11 deletions mockgcp/tools/gapic/pkg/protogen/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand All @@ -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, ".")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably check the tokens is the expected length.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call - done!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If its more than 2 doesn't that also suggest we may be splitting it incorrectly?

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),
Expand All @@ -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 {
Expand Down
26 changes: 26 additions & 0 deletions mockgcp/tools/gapic/pkg/protogen/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type ProtoWriter struct {
w io.Writer
errors []error
protoVersion int
comments *Comments
}

func NewProtoWriter(w io.Writer) *ProtoWriter {
Expand All @@ -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...)
Expand Down Expand Up @@ -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(" ")

Expand Down Expand Up @@ -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++ {
Expand Down
Loading