From e25da26ab6826c15b651aef0467ac5947627750f Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Thu, 19 Nov 2020 15:44:52 -0800 Subject: [PATCH] feat: generator options are parsed into a standalone struct (#479) Lifts some generator options into a separate structure. Lifts option parameter parsing logic into a standalone function. Adds parsing logic for permitted transports. Also adds unit tests for parsing transport options Default transport option is just grpc for now. --- internal/gengapic/doc_file.go | 6 +- internal/gengapic/doc_file_test.go | 3 +- internal/gengapic/gengapic.go | 177 ++++++++++++++++++++--------- internal/gengapic/gengapic_test.go | 86 ++++++++++++++ 4 files changed, 217 insertions(+), 55 deletions(-) diff --git a/internal/gengapic/doc_file.go b/internal/gengapic/doc_file.go index e4509e5d3..521f1cc8a 100644 --- a/internal/gengapic/doc_file.go +++ b/internal/gengapic/doc_file.go @@ -29,14 +29,14 @@ import ( // // Since it's the only file that needs to write package documentation and canonical import, // it does not use g.commit(). -func (g *generator) genDocFile(pkgPath, pkgName string, year int, scopes []string) { +func (g *generator) genDocFile(year int, scopes []string) { p := g.printf p(license.Apache, year) p("") if g.apiName != "" { - p("// Package %s is an auto-generated package for the", pkgName) + p("// Package %s is an auto-generated package for the ", g.opts.pkgName) p("// %s.", g.apiName) } @@ -73,7 +73,7 @@ func (g *generator) genDocFile(pkgPath, pkgName string, year int, scopes []strin p("//") p("// For information about setting deadlines, reusing contexts, and more") p("// please visit pkg.go.dev/cloud.google.com/go.") - p("package %s // import %q", pkgName, pkgPath) + p("package %s // import %q", g.opts.pkgName, g.opts.pkgPath) p("") p("import (") diff --git a/internal/gengapic/doc_file_test.go b/internal/gengapic/doc_file_test.go index d957871fc..133f2097e 100644 --- a/internal/gengapic/doc_file_test.go +++ b/internal/gengapic/doc_file_test.go @@ -29,6 +29,7 @@ func TestDocFile(t *testing.T) { Summary: "The Awesome Foo API is really really awesome. It enables the use of Foo with Buz and Baz to acclerate bar.", }, } + g.opts = &options{pkgPath: "path/to/awesome", pkgName: "awesome"} for _, tst := range []struct { relLvl, want string @@ -46,7 +47,7 @@ func TestDocFile(t *testing.T) { }, } { g.relLvl = tst.relLvl - g.genDocFile("path/to/awesome", "awesome", 42, []string{"https://foo.bar.com/auth", "https://zip.zap.com/auth"}) + g.genDocFile(42, []string{"https://foo.bar.com/auth", "https://zip.zap.com/auth"}) txtdiff.Diff(t, "doc_file", g.pt.String(), tst.want) g.reset() } diff --git a/internal/gengapic/gengapic.go b/internal/gengapic/gengapic.go index 76de18fb7..8df6486e3 100644 --- a/internal/gengapic/gengapic.go +++ b/internal/gengapic/gengapic.go @@ -26,8 +26,6 @@ import ( "unicode" "unicode/utf8" - yaml "gopkg.in/yaml.v2" - "github.com/golang/protobuf/proto" "github.com/golang/protobuf/protoc-gen-go/descriptor" plugin "github.com/golang/protobuf/protoc-gen-go/plugin" @@ -37,6 +35,7 @@ import ( "github.com/googleapis/gapic-generator-go/internal/pbinfo" "github.com/googleapis/gapic-generator-go/internal/printer" "google.golang.org/genproto/googleapis/api/annotations" + "gopkg.in/yaml.v2" ) const ( @@ -52,29 +51,65 @@ const ( var headerParamRegexp = regexp.MustCompile(`{([_.a-z]+)`) -func Gen(genReq *plugin.CodeGeneratorRequest) (*plugin.CodeGeneratorResponse, error) { - var pkgPath, pkgName, outDir string - var g generator +type Transport int + +const ( + grpc Transport = iota + rest +) + +type options struct { + pkgPath string + pkgName string + outDir string + relLvl string + modulePrefix string + grpcConfPath string + serviceConfigPath string + sampleOnly bool + transports []Transport +} + +// ParseOptions takes a string and parses it into a struct defining +// customizations on the target gapic surface. +// Options are comma-separated key/value pairs which are in turn delimited with '='. +// Valid options include: +// * go-gapic-package (package and module naming info) +// * sample-only (only checked for presence) +// * gapic-service-config (filepath) +// * grpc-service-config (filepath) +// * module (name) +// * release-level (one of 'alpha', 'beta', or empty) +// * transport ('+' separated list of transport backends to generate) +// The only required option is 'go-gapic-package'. +// +// Valid parameter example: +// go-gapic-package=path/to/out;pkg,module=path,transport=rest+grpc,gapic-service-config=gapic_cfg.json,release-level=alpha +// +// It returns a pointer to a populated options if no errors were encountered while parsing. +// If errors were encountered, it returns a nil pointer and the first error. +func ParseOptions(parameter *string) (*options, error) { + opts := options{sampleOnly: false} - if genReq.Parameter == nil { - return &g.resp, errors.E(nil, paramError) + if parameter == nil { + return nil, errors.E(nil, "empty options parameter") } // parse plugin params, ignoring unknown values - for _, s := range strings.Split(*genReq.Parameter, ",") { + for _, s := range strings.Split(*parameter, ",") { // check for the boolean flag, sample-only, that disables client generation if s == "sample-only" { - return &g.resp, nil + return &options{sampleOnly: true}, nil } e := strings.IndexByte(s, '=') if e < 0 { - return &g.resp, errors.E(nil, "invalid plugin option format, must be key=value: %s", s) + return nil, errors.E(nil, "invalid plugin option format, must be key=value: %s", s) } key, val := s[:e], s[e+1:] if val == "" { - return &g.resp, errors.E(nil, "invalid plugin option value, missing value in key=value: %s", s) + return nil, errors.E(nil, "invalid plugin option value, missing value in key=value: %s", s) } switch key { @@ -82,52 +117,88 @@ func Gen(genReq *plugin.CodeGeneratorRequest) (*plugin.CodeGeneratorResponse, er p := strings.IndexByte(s, ';') if p < 0 { - return &g.resp, errors.E(nil, paramError) + return nil, errors.E(nil, paramError) } - pkgPath = s[e+1 : p] - pkgName = s[p+1:] - outDir = filepath.FromSlash(pkgPath) + opts.pkgPath = s[e+1 : p] + opts.pkgName = s[p+1:] + opts.outDir = filepath.FromSlash(opts.pkgPath) case "gapic-service-config": - f, err := os.Open(val) - if err != nil { - return &g.resp, errors.E(nil, "error opening service config: %v", err) - } - - err = yaml.NewDecoder(f).Decode(&g.serviceConfig) - if err != nil { - return &g.resp, errors.E(nil, "error decoding service config: %v", err) - } + opts.serviceConfigPath = val case "grpc-service-config": - f, err := os.Open(val) - if err != nil { - return &g.resp, errors.E(nil, "error opening gRPC service config: %v", err) - } - - g.grpcConf, err = conf.New(f) - if err != nil { - return &g.resp, errors.E(nil, "error parsing gPRC service config: %v", err) - } + opts.grpcConfPath = val case "module": - g.modulePrefix = val + opts.modulePrefix = val case "release-level": - g.relLvl = strings.ToLower(val) + opts.relLvl = strings.ToLower(val) + case "transport": + for _, t := range strings.Split(val, "+") { + switch t { + case "grpc": + opts.transports = append(opts.transports, grpc) + case "rest": + opts.transports = append(opts.transports, rest) + default: + return nil, errors.E(nil, "invalid transport option: %s", t) + } + } } } - if pkgPath == "" || pkgName == "" || outDir == "" { - return &g.resp, errors.E(nil, paramError) + if opts.pkgPath == "" || opts.pkgName == "" || opts.outDir == "" { + return nil, errors.E(nil, paramError) } - if g.modulePrefix != "" { - if !strings.HasPrefix(outDir, g.modulePrefix) { - return &g.resp, errors.E(nil, "go-gapic-package %q does not match prefix %q", outDir, g.modulePrefix) + if opts.modulePrefix != "" { + if !strings.HasPrefix(opts.outDir, opts.modulePrefix) { + return nil, errors.E(nil, "go-gapic-package %q does not match prefix %q", opts.outDir, opts.modulePrefix) } - outDir = strings.TrimPrefix(outDir, g.modulePrefix+"/") + opts.outDir = strings.TrimPrefix(opts.outDir, opts.modulePrefix+"/") } + // Default is just grpc for now. + if opts.transports == nil { + opts.transports = []Transport{grpc} + } + + return &opts, nil +} + +func Gen(genReq *plugin.CodeGeneratorRequest) (*plugin.CodeGeneratorResponse, error) { + var g generator g.init(genReq.ProtoFile) + opts, err := ParseOptions(genReq.Parameter) + if err != nil { + return &g.resp, err + } + + if opts.serviceConfigPath != "" { + f, err := os.Open(opts.serviceConfigPath) + if err != nil { + return &g.resp, errors.E(nil, "error opening service config: %v", err) + } + defer f.Close() + + err = yaml.NewDecoder(f).Decode(&g.serviceConfig) + if err != nil { + return &g.resp, errors.E(nil, "error decoding service config: %v", err) + } + } + if opts.grpcConfPath != "" { + f, err := os.Open(opts.grpcConfPath) + if err != nil { + return &g.resp, errors.E(nil, "error opening gRPC service config: %v", err) + } + defer f.Close() + + g.grpcConf, err = conf.New(f) + if err != nil { + return &g.resp, errors.E(nil, "error parsing gPRC service config: %v", err) + } + } + g.opts = opts + var genServs []*descriptor.ServiceDescriptorProto for _, f := range genReq.ProtoFile { if !strContains(genReq.FileToGenerate, f.GetName()) { @@ -149,20 +220,20 @@ func Gen(genReq *plugin.CodeGeneratorRequest) (*plugin.CodeGeneratorResponse, er // Keep the current behavior for now, but we could revisit this later. outFile := pbinfo.ReduceServName(s.GetName(), "") outFile = camelToSnake(outFile) - outFile = filepath.Join(outDir, outFile) + outFile = filepath.Join(g.opts.outDir, outFile) g.reset() - if err := g.gen(s, pkgName); err != nil { + if err := g.gen(s); err != nil { return &g.resp, err } - g.commit(outFile+"_client.go", pkgName) + g.commit(outFile+"_client.go", g.opts.pkgName) g.reset() - if err := g.genExampleFile(s, pkgName); err != nil { + if err := g.genExampleFile(s, g.opts.pkgName); err != nil { return &g.resp, errors.E(err, "example: %s", s.GetName()) } - g.imports[pbinfo.ImportSpec{Name: pkgName, Path: pkgPath}] = true - g.commit(outFile+"_client_example_test.go", pkgName+"_test") + g.imports[pbinfo.ImportSpec{Name: g.opts.pkgName, Path: g.opts.pkgPath}] = true + g.commit(outFile+"_client_example_test.go", g.opts.pkgName+"_test") } g.reset() @@ -170,9 +241,9 @@ func Gen(genReq *plugin.CodeGeneratorRequest) (*plugin.CodeGeneratorResponse, er if err != nil { return &g.resp, err } - g.genDocFile(pkgPath, pkgName, time.Now().Year(), scopes) + g.genDocFile(time.Now().Year(), scopes) g.resp.File = append(g.resp.File, &plugin.CodeGeneratorResponse_File{ - Name: proto.String(filepath.Join(outDir, "doc.go")), + Name: proto.String(filepath.Join(g.opts.outDir, "doc.go")), Content: proto.String(g.pt.String()), }) @@ -218,6 +289,10 @@ type generator struct { // The Go module prefix to strip from the go-gapic-package // used as the generated file name. modulePrefix string + + // Options for the generator determining module names, transports, + // config file paths, etc. + opts *options } func (g *generator) init(files []*descriptor.FileDescriptorProto) { @@ -330,8 +405,8 @@ func (g *generator) reset() { } // gen generates client for the given service. -func (g *generator) gen(serv *descriptor.ServiceDescriptorProto, pkgName string) error { - servName := pbinfo.ReduceServName(*serv.Name, pkgName) +func (g *generator) gen(serv *descriptor.ServiceDescriptorProto) error { + servName := pbinfo.ReduceServName(*serv.Name, g.opts.pkgName) g.clientHook(servName) if err := g.clientOptions(serv, servName); err != nil { diff --git a/internal/gengapic/gengapic_test.go b/internal/gengapic/gengapic_test.go index afdff90dc..122e4a384 100644 --- a/internal/gengapic/gengapic_test.go +++ b/internal/gengapic/gengapic_test.go @@ -17,6 +17,7 @@ package gengapic import ( "bytes" "path/filepath" + "reflect" "testing" "github.com/golang/protobuf/proto" @@ -499,3 +500,88 @@ func Test_isLRO(t *testing.T) { } } } + +func Test_optionsParse(t *testing.T) { + for _, tst := range []struct { + param string + expectedOpts *options + expectErr bool + }{ + { + param: "transport=grpc,go-gapic-package=path;pkg", + expectedOpts: &options{ + transports: []Transport{grpc}, + pkgPath: "path", + pkgName: "pkg", + outDir: "path", + }, + expectErr: false, + }, + { + param: "transport=rest+grpc,go-gapic-package=path;pkg", + expectedOpts: &options{ + transports: []Transport{rest, grpc}, + pkgPath: "path", + pkgName: "pkg", + outDir: "path", + }, + expectErr: false, + }, + { + param: "go-gapic-package=path;pkg", + expectedOpts: &options{ + transports: []Transport{grpc}, + pkgPath: "path", + pkgName: "pkg", + outDir: "path", + }, + }, + { + param: "module=path,go-gapic-package=path/to/out;pkg", + expectedOpts: &options{ + transports: []Transport{grpc}, + pkgPath: "path/to/out", + pkgName: "pkg", + outDir: "to/out", + modulePrefix: "path", + }, + expectErr: false, + }, + { + param: "transport=tcp,go-gapic-package=path;pkg", + expectErr: true, + }, + { + param: "go-gapic-package=pkg;", + expectErr: true, + }, + { + param: "go-gapic-package=;path", + expectErr: true, + }, + { + param: "go-gapic-package=bogus", + expectErr: true, + }, + { + param: "module=different_path,go-gapic-package=path;pkg", + expectErr: true, + }, + } { + opts, err := ParseOptions(&tst.param) + if tst.expectErr && err == nil { + t.Errorf("ParseOptions(%s) expected error", tst.param) + continue + } + + if !tst.expectErr && err != nil { + t.Errorf("ParseOptions(%s) got unexpected error: %v", tst.param, err) + continue + } + + if !reflect.DeepEqual(opts, tst.expectedOpts) { + t.Errorf("ParseOptions(%s) = %v, expected %v", tst.param, opts, tst.expectedOpts) + continue + } + } +}