From eb841d09882a850efbae059697a86c0aefc01c99 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 15 Jan 2026 17:41:53 +0000 Subject: [PATCH 1/3] feat: refactor generator options -> config --- internal/gengapic/auxiliary.go | 6 +- internal/gengapic/auxiliary_test.go | 2 +- internal/gengapic/client_init.go | 4 +- internal/gengapic/client_init_test.go | 37 ++++---- internal/gengapic/custom_operation.go | 4 +- internal/gengapic/custom_operation_test.go | 2 +- internal/gengapic/doc_file.go | 18 ++-- internal/gengapic/doc_file_test.go | 52 +++++------ internal/gengapic/example.go | 12 +-- internal/gengapic/example_test.go | 55 ++++++----- internal/gengapic/generator.go | 67 +++----------- internal/gengapic/generator_test.go | 4 +- internal/gengapic/gengapic.go | 51 +++++------ internal/gengapic/gengapic_test.go | 102 +++++++++++---------- internal/gengapic/gengrpc.go | 6 +- internal/gengapic/gengrpc_test.go | 26 +++--- internal/gengapic/genrest.go | 14 +-- internal/gengapic/genrest_test.go | 62 ++++++------- internal/gengapic/lro.go | 2 +- internal/gengapic/mixins.go | 20 ++-- internal/gengapic/mixins_test.go | 86 +++++++++-------- internal/gengapic/options.go | 101 ++++++++++++++------ internal/gengapic/options_test.go | 47 +++++----- internal/gengapic/paging_test.go | 4 +- internal/gengapic/snippets.go | 26 +++--- internal/gengapic/snippets_test.go | 12 +-- 26 files changed, 422 insertions(+), 400 deletions(-) diff --git a/internal/gengapic/auxiliary.go b/internal/gengapic/auxiliary.go index 6a151d32e6a..d30a3d2b9a7 100644 --- a/internal/gengapic/auxiliary.go +++ b/internal/gengapic/auxiliary.go @@ -76,11 +76,11 @@ func (g *generator) genAuxFile() error { return err } - g.commit(filepath.Join(g.opts.outDir, "auxiliary.go"), g.opts.pkgName) + g.commit(filepath.Join(g.cfg.outDir, "auxiliary.go"), g.cfg.pkgName) g.reset() g.genIteratorsGo123() - g.commitWithBuildTag(filepath.Join(g.opts.outDir, "auxiliary_go123.go"), g.opts.pkgName, "go1.23") + g.commitWithBuildTag(filepath.Join(g.cfg.outDir, "auxiliary_go123.go"), g.cfg.pkgName, "go1.23") g.reset() return nil @@ -251,7 +251,7 @@ func (g *generator) maybeAddOperationWrapper(m *descriptorpb.MethodDescriptorPro // this. func (g *generator) genOperationWrapperType(ow operationWrapper) error { p := g.pt.Printf - hasREST := containsTransport(g.opts.transports, rest) + hasREST := containsTransport(g.cfg.transports, rest) isEmpty := ow.responseName == emptyValue // Response Go type resolution. diff --git a/internal/gengapic/auxiliary_test.go b/internal/gengapic/auxiliary_test.go index 2ee7198d270..caf21c16438 100644 --- a/internal/gengapic/auxiliary_test.go +++ b/internal/gengapic/auxiliary_test.go @@ -392,7 +392,7 @@ func TestGenOperations(t *testing.T) { ParentElement: make(map[pbinfo.ProtoType]pbinfo.ProtoType), }, imports: make(map[pbinfo.ImportSpec]bool), - opts: &options{transports: []transport{grpc, rest}}, + cfg: &generatorConfig{transports: []transport{grpc, rest}}, } wantImports := map[pbinfo.ImportSpec]bool{ diff --git a/internal/gengapic/client_init.go b/internal/gengapic/client_init.go index 1e1be2a5574..4476868a3c8 100644 --- a/internal/gengapic/client_init.go +++ b/internal/gengapic/client_init.go @@ -54,7 +54,7 @@ func (g *generator) clientOptions(serv *descriptorpb.ServiceDescriptorProto, ser g.imports[pbinfo.ImportSpec{Name: "gax", Path: "github.com/googleapis/gax-go/v2"}] = true } - for _, v := range g.opts.transports { + for _, v := range g.cfg.transports { switch v { case grpc: if err := g.grpcClientOptions(serv, servName); err != nil { @@ -396,7 +396,7 @@ func (g *generator) makeClients(serv *descriptorpb.ServiceDescriptorProto, servN } g.clientInit(serv, servName, hasLRO) - for _, v := range g.opts.transports { + for _, v := range g.cfg.transports { switch v { case grpc: g.grpcClientInit(serv, servName, imp, hasLRO) diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index fdb0dfa8df9..39fc8828899 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -122,17 +122,19 @@ func TestClientOpt(t *testing.T) { "google.cloud.location.Locations": locationMethods(), "google.iam.v1.IAMPolicy": iamPolicyMethods(), }, - serviceConfig: &serviceconfig.Service{ - Name: "showcase.googleapis.com", - Apis: []*apipb.Api{ - {Name: "foo.bar.Baz"}, - {Name: "google.iam.v1.IAMPolicy"}, - {Name: "google.cloud.location.Locations"}, - {Name: "google.longrunning.Operations"}, + + cfg: &generatorConfig{ + transports: []transport{grpc, rest}, + APIServiceConfig: &serviceconfig.Service{ + Name: "showcase.googleapis.com", + Apis: []*apipb.Api{ + {Name: "foo.bar.Baz"}, + {Name: "google.iam.v1.IAMPolicy"}, + {Name: "google.cloud.location.Locations"}, + {Name: "google.longrunning.Operations"}, + }, }, - }, - opts: &options{transports: []transport{grpc, rest}}, - grpcConf: grpcConf, + gRPCServiceConfig: grpcConf}, } serv := &descriptorpb.ServiceDescriptorProto{ @@ -270,6 +272,7 @@ func TestServiceDoc(t *testing.T) { var g generator g.comments = make(map[protoiface.MessageV1]string) + g.cfg = &generatorConfig{} for _, tst := range []struct { in, want string @@ -573,12 +576,14 @@ func TestClientInit(t *testing.T) { tst.serv.GetMethod()[0]: "Does some stuff.", } g.mixins = tst.mixins - g.serviceConfig = &serviceconfig.Service{ - Apis: []*apipb.Api{ - {Name: "foo.bar.Baz"}, - {Name: "google.iam.v1.IAMPolicy"}, - {Name: "google.cloud.location.Locations"}, - {Name: "google.longrunning.Operations"}, + g.cfg = &generatorConfig{ + APIServiceConfig: &serviceconfig.Service{ + Apis: []*apipb.Api{ + {Name: "foo.bar.Baz"}, + {Name: "google.iam.v1.IAMPolicy"}, + {Name: "google.cloud.location.Locations"}, + {Name: "google.longrunning.Operations"}, + }, }, } g.aux.customOp = &customOp{ diff --git a/internal/gengapic/custom_operation.go b/internal/gengapic/custom_operation.go index 8b33d2a8ebf..c9a114f256d 100644 --- a/internal/gengapic/custom_operation.go +++ b/internal/gengapic/custom_operation.go @@ -33,7 +33,7 @@ type customOp struct { // isCustomOp determines if the given method should return a custom operation wrapper. func (g *generator) isCustomOp(m *descriptorpb.MethodDescriptorProto, info *httpInfo) bool { - return g.opts.generateAsDIREGAPIC && // Generator in DIREGAPIC mode. + return g.cfg.generateAsDIREGAPIC && // Generator in DIREGAPIC mode. g.aux.customOp != nil && // API Defines a custom operation. m.GetOutputType() == g.customOpProtoName() && // Method returns the custom operation. m.GetName() != "Wait" && // Method is not a Wait (uses POST). @@ -70,7 +70,7 @@ func (g *generator) customOpPointerType() (string, error) { // operation wrapper type with the Go identifier for a variable that is the // proto-defined operation type. func (g *generator) customOpInit(rspVar, reqVar, opVar string, req *descriptorpb.DescriptorProto, s *descriptorpb.ServiceDescriptorProto) { - h := handleName(s.GetName(), g.opts.pkgName) + h := handleName(s.GetName(), g.cfg.pkgName) opName := g.aux.customOp.message.GetName() pt := g.pt.Printf diff --git a/internal/gengapic/custom_operation_test.go b/internal/gengapic/custom_operation_test.go index 19d4413cf80..648f3618c35 100644 --- a/internal/gengapic/custom_operation_test.go +++ b/internal/gengapic/custom_operation_test.go @@ -114,7 +114,7 @@ func TestCustomOpInit(t *testing.T) { }, }, }, - opts: &options{pkgName: "bar"}, + cfg: &generatorConfig{pkgName: "bar"}, } g.customOpInit("foo", "req", "op", req, opServ) txtdiff.Diff(t, g.pt.String(), filepath.Join("testdata", "custom_op_init_helper.want")) diff --git a/internal/gengapic/doc_file.go b/internal/gengapic/doc_file.go index b62e308363c..d52f4eee3ca 100644 --- a/internal/gengapic/doc_file.go +++ b/internal/gengapic/doc_file.go @@ -41,12 +41,12 @@ func (g *generator) genDocFile(year int, services []*descriptorpb.ServiceDescrip p("") if g.apiName != "" { - p("// Package %s is an auto-generated package for the ", g.opts.pkgName) + p("// Package %s is an auto-generated package for the ", g.cfg.pkgName) p("// %s.", g.apiName) } - if g.serviceConfig != nil && g.serviceConfig.GetDocumentation() != nil { - summary := g.serviceConfig.GetDocumentation().GetSummary() + if g.cfg.APIServiceConfig != nil && g.cfg.APIServiceConfig.GetDocumentation() != nil { + summary := g.cfg.APIServiceConfig.GetDocumentation().GetSummary() summary = mdPlain(summary) wrapped := wrapString(summary, 75) @@ -59,7 +59,7 @@ func (g *generator) genDocFile(year int, services []*descriptorpb.ServiceDescrip } } - switch g.opts.relLvl { + switch g.cfg.relLvl { case alpha: p("//") p("// NOTE: This package is in alpha. It is not stable, and is likely to change.") @@ -92,10 +92,10 @@ func (g *generator) genDocFile(year int, services []*descriptorpb.ServiceDescrip // Code block for client creation exampleService := services[0] override := g.getServiceNameOverride(exampleService) - servName := pbinfo.ReduceServNameWithOverride(exampleService.GetName(), g.opts.pkgName, override) + servName := pbinfo.ReduceServNameWithOverride(exampleService.GetName(), g.cfg.pkgName, override) tmpClient := g.pt g.pt = printer.P{} - g.exampleInitClientWithOpts(g.opts.pkgName, servName, true) + g.exampleInitClientWithOpts(g.cfg.pkgName, servName, true) snipClient := g.pt.String() g.pt = tmpClient g.codesnippet(snipClient) @@ -112,7 +112,7 @@ func (g *generator) genDocFile(year int, services []*descriptorpb.ServiceDescrip // Code block for client using the first method of the service tmpMethod := g.pt g.pt = printer.P{} - g.exampleMethodBodyWithOpts(g.opts.pkgName, servName, exampleService.GetMethod()[0], true) + g.exampleMethodBodyWithOpts(g.cfg.pkgName, servName, exampleService.GetMethod()[0], true) snipMethod := g.pt.String() g.pt = tmpMethod g.codesnippet(snipMethod) @@ -131,7 +131,7 @@ func (g *generator) genDocFile(year int, services []*descriptorpb.ServiceDescrip p("// [Testing against Client Libraries]: https://pkg.go.dev/cloud.google.com/go#hdr-Testing") p("// [Debugging Client Libraries]: https://pkg.go.dev/cloud.google.com/go#hdr-Debugging") p("// [Inspecting errors]: https://pkg.go.dev/cloud.google.com/go#hdr-Inspecting_errors") - p("package %s // import %q", g.opts.pkgName, g.opts.pkgPath) + p("package %s // import %q", g.cfg.pkgName, g.cfg.pkgPath) p("") } @@ -202,7 +202,7 @@ func (g *generator) apiVersionSection(services []*descriptorpb.ServiceDescriptor // Construct the reduced/overridden service name used for client // type name derivation. override := g.getServiceNameOverride(s) - sn := pbinfo.ReduceServNameWithOverride(n, g.opts.pkgName, override) + sn := pbinfo.ReduceServNameWithOverride(n, g.cfg.pkgName, override) ct := fmt.Sprintf("%sClient", sn) // Use the raw proto service name in the tuple to associate it with diff --git a/internal/gengapic/doc_file_test.go b/internal/gengapic/doc_file_test.go index 9d6ca55173c..f56d553dba2 100644 --- a/internal/gengapic/doc_file_test.go +++ b/internal/gengapic/doc_file_test.go @@ -27,13 +27,13 @@ import ( func TestDocFile(t *testing.T) { g := generator{ - apiName: sample.ServiceTitle, - serviceConfig: sample.ServiceConfig(), - imports: map[pbinfo.ImportSpec]bool{}, - opts: &options{ - pkgPath: sample.GoPackagePath, - pkgName: sample.GoPackageName, - transports: []transport{grpc, rest}, + apiName: sample.ServiceTitle, + imports: map[pbinfo.ImportSpec]bool{}, + cfg: &generatorConfig{ + pkgPath: sample.GoPackagePath, + pkgName: sample.GoPackageName, + transports: []transport{grpc, rest}, + APIServiceConfig: sample.ServiceConfig(), }, } @@ -67,7 +67,7 @@ func TestDocFile(t *testing.T) { }, } { t.Run(tst.want, func(t *testing.T) { - g.opts.relLvl = tst.relLvl + g.cfg.relLvl = tst.relLvl g.genDocFile(sample.Year, []*descriptorpb.ServiceDescriptorProto{serv}) txtdiff.Diff(t, g.pt.String(), tst.want) g.reset() @@ -77,13 +77,13 @@ func TestDocFile(t *testing.T) { func TestDocFile_APIVersionSection(t *testing.T) { g := generator{ - apiName: sample.ServiceTitle, - serviceConfig: sample.ServiceConfig(), - imports: map[pbinfo.ImportSpec]bool{}, - opts: &options{ - pkgPath: sample.GoPackagePath, - pkgName: sample.GoPackageName, - transports: []transport{grpc, rest}, + apiName: sample.ServiceTitle, + imports: map[pbinfo.ImportSpec]bool{}, + cfg: &generatorConfig{ + pkgPath: sample.GoPackagePath, + pkgName: sample.GoPackageName, + transports: []transport{grpc, rest}, + APIServiceConfig: sample.ServiceConfig(), }, } @@ -110,13 +110,13 @@ func TestDocFile_APIVersionSection(t *testing.T) { func TestDocFileEmptyService(t *testing.T) { g := generator{ - apiName: sample.ServiceTitle, - imports: map[pbinfo.ImportSpec]bool{}, - serviceConfig: sample.ServiceConfig(), - opts: &options{ - pkgPath: sample.GoPackagePath, - pkgName: sample.GoPackageName, - transports: []transport{grpc, rest}, + apiName: sample.ServiceTitle, + imports: map[pbinfo.ImportSpec]bool{}, + cfg: &generatorConfig{ + pkgPath: sample.GoPackagePath, + pkgName: sample.GoPackageName, + transports: []transport{grpc, rest}, + APIServiceConfig: sample.ServiceConfig(), }, } inputType := sample.InputType(sample.CreateRequest) @@ -154,7 +154,7 @@ func TestDocFileEmptyService(t *testing.T) { }, } { t.Run(tst.want, func(t *testing.T) { - g.opts.relLvl = tst.relLvl + g.cfg.relLvl = tst.relLvl g.genDocFile(sample.Year, []*descriptorpb.ServiceDescriptorProto{serv}) txtdiff.Diff(t, g.pt.String(), tst.want) g.reset() @@ -164,9 +164,9 @@ func TestDocFileEmptyService(t *testing.T) { func TestApiVersionSection(t *testing.T) { g := generator{ - serviceConfig: sample.ServiceConfig(), - opts: &options{ - pkgName: sample.GoPackageName, + cfg: &generatorConfig{ + pkgName: sample.GoPackageName, + APIServiceConfig: sample.ServiceConfig(), }, } diff --git a/internal/gengapic/example.go b/internal/gengapic/example.go index 218c98b7c9e..42375fb3d6c 100644 --- a/internal/gengapic/example.go +++ b/internal/gengapic/example.go @@ -25,7 +25,7 @@ import ( ) func (g *generator) genExampleFile(serv *descriptorpb.ServiceDescriptorProto) error { - pkgName := g.opts.pkgName + pkgName := g.cfg.pkgName override := g.getServiceNameOverride(serv) servName := pbinfo.ReduceServNameWithOverride(serv.GetName(), pkgName, override) @@ -42,7 +42,7 @@ func (g *generator) genExampleFile(serv *descriptorpb.ServiceDescriptorProto) er } func (g *generator) genExampleIteratorFile(serv *descriptorpb.ServiceDescriptorProto) error { - pkgName := g.opts.pkgName + pkgName := g.cfg.pkgName override := g.getServiceNameOverride(serv) servName := pbinfo.ReduceServNameWithOverride(serv.GetName(), pkgName, override) methods := append(serv.GetMethod(), g.getMixinMethods()...) @@ -75,7 +75,7 @@ func (g *generator) genExampleIteratorFile(serv *descriptorpb.ServiceDescriptorP g.imports[inSpec] = true // Pick the first transport for simplicity. We don't need examples // of each method for both transports when they have the same surface. - t := g.opts.transports[0] + t := g.cfg.transports[0] s := servName if t == rest { s += "REST" @@ -100,7 +100,7 @@ func (g *generator) genExampleIteratorFile(serv *descriptorpb.ServiceDescriptorP func (g *generator) exampleClientFactory(pkgName, servName string) { p := g.printf - for _, t := range g.opts.transports { + for _, t := range g.cfg.transports { s := servName if t == rest { s += "REST" @@ -125,7 +125,7 @@ func (g *generator) exampleInitClient(pkgName, servName string) { func (g *generator) exampleInitClientWithOpts(pkgName, servName string, isPackageDoc bool) { p := g.printf if isPackageDoc { - p("// go get %s@latest", g.opts.pkgPath) + p("// go get %s@latest", g.cfg.pkgPath) } p("ctx := context.Background()") p("// This snippet has been automatically generated and should be regarded as a code template only.") @@ -195,7 +195,7 @@ func (g *generator) exampleMethodBodyWithOpts(pkgName, servName string, m *descr g.imports[inSpec] = true // Pick the first transport for simplicity. We don't need examples // of each method for both transports when they have the same surface. - t := g.opts.transports[0] + t := g.cfg.transports[0] s := servName if t == rest { s += "REST" diff --git a/internal/gengapic/example_test.go b/internal/gengapic/example_test.go index fafd168f914..0cd52679fef 100644 --- a/internal/gengapic/example_test.go +++ b/internal/gengapic/example_test.go @@ -41,7 +41,7 @@ func TestExample(t *testing.T) { "google.iam.v1.IAMPolicy": iamPolicyMethods(), } g.mixins = mix - g.serviceConfig = &serviceconfig.Service{ + apiCfg := &serviceconfig.Service{ Apis: []*apipb.Api{ {Name: "foo.bar.Baz"}, {Name: "google.iam.v1.IAMPolicy"}, @@ -176,14 +176,15 @@ func TestExample(t *testing.T) { for _, tst := range []struct { tstName string imports map[pbinfo.ImportSpec]bool - options options + cfg *generatorConfig op *customOp }{ { tstName: "empty_example", - options: options{ - pkgName: "Foo", - transports: []transport{grpc, rest}, + cfg: &generatorConfig{ + pkgName: "Foo", + transports: []transport{grpc, rest}, + APIServiceConfig: apiCfg, }, imports: map[pbinfo.ImportSpec]bool{ {Path: "context"}: true, @@ -197,9 +198,10 @@ func TestExample(t *testing.T) { }, { tstName: "empty_example_grpc", - options: options{ - pkgName: "Foo", - transports: []transport{grpc}, + cfg: &generatorConfig{ + pkgName: "Foo", + transports: []transport{grpc}, + APIServiceConfig: apiCfg, }, imports: map[pbinfo.ImportSpec]bool{ {Path: "context"}: true, @@ -213,9 +215,10 @@ func TestExample(t *testing.T) { }, { tstName: "foo_example", - options: options{ - pkgName: "Bar", - transports: []transport{grpc, rest}, + cfg: &generatorConfig{ + pkgName: "Bar", + transports: []transport{grpc, rest}, + APIServiceConfig: apiCfg, }, imports: map[pbinfo.ImportSpec]bool{ {Path: "context"}: true, @@ -229,9 +232,10 @@ func TestExample(t *testing.T) { }, { tstName: "foo_example_rest", - options: options{ - pkgName: "Bar", - transports: []transport{rest}, + cfg: &generatorConfig{ + pkgName: "Bar", + transports: []transport{rest}, + APIServiceConfig: apiCfg, }, imports: map[pbinfo.ImportSpec]bool{ {Path: "context"}: true, @@ -245,10 +249,11 @@ func TestExample(t *testing.T) { }, { tstName: "custom_op_example", - options: options{ + cfg: &generatorConfig{ pkgName: "Bar", transports: []transport{rest}, generateAsDIREGAPIC: true, + APIServiceConfig: apiCfg, }, imports: map[pbinfo.ImportSpec]bool{ {Path: "context"}: true, @@ -261,9 +266,9 @@ func TestExample(t *testing.T) { } { t.Run(tst.tstName, func(t *testing.T) { g.reset() - g.opts = &tst.options + g.cfg = tst.cfg g.mixins = mix - if tst.options.generateAsDIREGAPIC { + if tst.cfg.generateAsDIREGAPIC { g.mixins = nil } g.aux.customOp = tst.op @@ -279,9 +284,9 @@ func TestExample(t *testing.T) { delete(tst.imports, pbinfo.ImportSpec{Path: "io"}) delete(tst.imports, pbinfo.ImportSpec{Name: "iampb", Path: "cloud.google.com/go/iam/apiv1/iampb"}) - g.opts = &tst.options + g.cfg = tst.cfg g.mixins = mix - if tst.options.generateAsDIREGAPIC { + if tst.cfg.generateAsDIREGAPIC { g.mixins = nil } g.aux.customOp = tst.op @@ -297,7 +302,6 @@ func TestExample(t *testing.T) { func TestGenSnippetFile(t *testing.T) { var g generator g.imports = map[pbinfo.ImportSpec]bool{} - g.serviceConfig = sample.ServiceConfig() serv := sample.Service() g.snippetMetadata = snippets.NewMetadata(sample.ProtoPackagePath, sample.GoPackagePath, sample.GoPackageName) @@ -319,14 +323,15 @@ func TestGenSnippetFile(t *testing.T) { for _, test := range []struct { name string - options options + cfg *generatorConfig imports map[pbinfo.ImportSpec]bool }{ { name: "snippet", - options: options{ - pkgName: sample.GoPackageName, - transports: []transport{grpc, rest}, + cfg: &generatorConfig{ + pkgName: sample.GoPackageName, + transports: []transport{grpc, rest}, + APIServiceConfig: sample.ServiceConfig(), }, imports: map[pbinfo.ImportSpec]bool{ {Path: "context"}: true, @@ -336,7 +341,7 @@ func TestGenSnippetFile(t *testing.T) { } { t.Run(test.name, func(t *testing.T) { g.reset() - g.opts = &test.options + g.cfg = test.cfg err := g.genSnippetFile(serv, serv.Method[0]) if err != nil { t.Fatal(err) diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index e84d70931ea..d1f0d9490d0 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -16,20 +16,15 @@ package gengapic import ( "fmt" - "os" "strings" "time" - "github.com/ghodss/yaml" - conf "github.com/googleapis/gapic-generator-go/internal/grpc_service_config" "github.com/googleapis/gapic-generator-go/internal/license" "github.com/googleapis/gapic-generator-go/internal/pbinfo" "github.com/googleapis/gapic-generator-go/internal/printer" "github.com/googleapis/gapic-generator-go/internal/snippets" "google.golang.org/genproto/googleapis/api/annotations" - "google.golang.org/genproto/googleapis/api/serviceconfig" "google.golang.org/genproto/googleapis/gapic/metadata" - "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/runtime/protoiface" "google.golang.org/protobuf/types/descriptorpb" @@ -88,18 +83,12 @@ type generator struct { // Human-readable name of the API used in docs apiName string - // Parsed service config from plugin option - serviceConfig *serviceconfig.Service - - // gRPC ServiceConfig - grpcConf conf.Config - // Auxiliary types to be generated in the package aux *auxTypes // Options for the generator determining module names, transports, // config file paths, etc. - opts *options + cfg *generatorConfig // GapicMetadata for recording proto-to-code mappings in a // gapic_metadata.json file. @@ -136,56 +125,22 @@ func newGenerator(req *pluginpb.CodeGeneratorRequest) (*generator, error) { }, } - opts, err := newOptionsFromParams(req.Parameter) + // Build and validate the immutable configuration from the CodeGeneratorRequest plugin args. + cfg, err := configFromRequest(req.Parameter) if err != nil { return nil, err } + g.cfg = cfg + files := req.GetProtoFile() files = append(files, wellKnownTypeFiles...) - if opts.serviceConfigPath != "" { - y, err := os.ReadFile(opts.serviceConfigPath) - if err != nil { - return nil, fmt.Errorf("error reading service config: %v", err) - } - - j, err := yaml.YAMLToJSON(y) - if err != nil { - return nil, fmt.Errorf("error converting YAML to JSON: %v", err) - } - - cfg := &serviceconfig.Service{} - if err := (protojson.UnmarshalOptions{DiscardUnknown: true}).Unmarshal(j, cfg); err != nil { - return nil, fmt.Errorf("error unmarshaling service config: %v", err) - } - g.serviceConfig = cfg - - // An API Service Config will always have a `name` so if it is not populated, - // it's an invalid config. - if g.serviceConfig.GetName() == "" { - return nil, fmt.Errorf("invalid API service config file %q", opts.serviceConfigPath) - } - - g.collectMixins() - files = append(files, g.getMixinFiles()...) - } - if opts.grpcConfPath != "" { - f, err := os.Open(opts.grpcConfPath) - if err != nil { - return nil, fmt.Errorf("error opening gRPC service config: %v", err) - } - defer f.Close() - - g.grpcConf, err = conf.New(f) - if err != nil { - return nil, fmt.Errorf("error parsing gPRC service config: %v", err) - } - } - g.opts = opts + g.collectMixins() + files = append(files, g.getMixinFiles()...) g.descInfo = pbinfo.Of(files) - if len(g.opts.pkgOverrides) > 0 { - g.descInfo.PkgOverrides = g.opts.pkgOverrides + if len(g.cfg.pkgOverrides) > 0 { + g.descInfo.PkgOverrides = g.cfg.pkgOverrides } for _, f := range files { @@ -357,7 +312,7 @@ func (g *generator) autoPopulatedFields(_ string, m *descriptorpb.MethodDescript var apfs []string // Find the service config's AutoPopulatedFields entry by method name. mfqn := g.fqn(m) - for _, s := range g.serviceConfig.GetPublishing().GetMethodSettings() { + for _, s := range g.cfg.APIServiceConfig.GetPublishing().GetMethodSettings() { if s.GetSelector() == mfqn { apfs = s.AutoPopulatedFields break @@ -381,7 +336,7 @@ func (g *generator) autoPopulatedFields(_ string, m *descriptorpb.MethodDescript // getServiceNameOverride checks to see if the service has a defined service name override. func (g *generator) getServiceNameOverride(s *descriptorpb.ServiceDescriptorProto) string { - ls := g.serviceConfig.GetPublishing().GetLibrarySettings() + ls := g.cfg.APIServiceConfig.GetPublishing().GetLibrarySettings() if len(ls) == 0 { return "" } diff --git a/internal/gengapic/generator_test.go b/internal/gengapic/generator_test.go index 5c4178229bb..4a0c49196d9 100644 --- a/internal/gengapic/generator_test.go +++ b/internal/gengapic/generator_test.go @@ -92,12 +92,12 @@ func TestAutoPopulatedFields(t *testing.T) { } var g generator - g.opts = &options{ + g.cfg = &generatorConfig{ pkgName: "pkg", } g.imports = map[pbinfo.ImportSpec]bool{} - g.serviceConfig = &serviceconfig.Service{ + g.cfg.APIServiceConfig = &serviceconfig.Service{ Publishing: &annotations.Publishing{ MethodSettings: []*annotations.MethodSettings{ { diff --git a/internal/gengapic/gengapic.go b/internal/gengapic/gengapic.go index a963d3a5be1..fc36fd4407b 100644 --- a/internal/gengapic/gengapic.go +++ b/internal/gengapic/gengapic.go @@ -40,7 +40,6 @@ const ( alpha = "alpha" beta = "beta" deprecated = "deprecated" - disableDeadlinesVar = "GOOGLE_API_GO_EXPERIMENTAL_DISABLE_DEFAULT_DEADLINE" fieldTypeBool = descriptorpb.FieldDescriptorProto_TYPE_BOOL fieldTypeString = descriptorpb.FieldDescriptorProto_TYPE_STRING fieldTypeBytes = descriptorpb.FieldDescriptorProto_TYPE_BYTES @@ -79,13 +78,13 @@ func gen(genReq *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse g.hasIAMPolicyOverrides = true } - if g.serviceConfig != nil { - g.apiName = g.serviceConfig.GetTitle() + if g.cfg.APIServiceConfig != nil { + g.apiName = g.cfg.APIServiceConfig.GetTitle() } protoPkg := g.descInfo.ParentFile[genServs[0]].GetPackage() - if op, ok := g.descInfo.Type[fmt.Sprintf(".%s.Operation", protoPkg)]; g.opts.generateAsDIREGAPIC && ok { + if op, ok := g.descInfo.Type[fmt.Sprintf(".%s.Operation", protoPkg)]; g.cfg.generateAsDIREGAPIC && ok { g.aux.customOp = &customOp{ message: op.(*descriptorpb.DescriptorProto), handles: []*descriptorpb.ServiceDescriptorProto{}, @@ -97,7 +96,7 @@ func gen(genReq *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse if len(genServs) > 0 { g.metadata.ProtoPackage = protoPkg } - g.metadata.LibraryPackage = g.opts.pkgPath + g.metadata.LibraryPackage = g.cfg.pkgPath // Initialize the model that will collect snippet metadata. g.snippetMetadata = g.newSnippetsMetadata(protoPkg) @@ -114,7 +113,7 @@ func gen(genReq *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse override := g.getServiceNameOverride(s) servName := pbinfo.ReduceServNameWithOverride(s.GetName(), "", override) outFile := camelToSnake(servName) - outFile = filepath.Join(g.opts.outDir, outFile) + outFile = filepath.Join(g.cfg.outDir, outFile) if err := g.genAndCommitSnippets(s); err != nil { return &g.resp, fmt.Errorf("error generating snippets for %s: %v ", s.GetName(), err) @@ -123,34 +122,34 @@ func gen(genReq *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse g.reset() // If the service has no REST-able RPCs, then a REGAPIC should not be // generated for it, even if REST is an enabled transport. - transports := g.opts.transports + transports := g.cfg.transports hasREST := hasRESTMethod(s) if !hasREST { - g.opts.transports = []transport{grpc} + g.cfg.transports = []transport{grpc} } if err := g.gen(s); err != nil { return nil, err } - g.commit(outFile+"_client.go", g.opts.pkgName) + g.commit(outFile+"_client.go", g.cfg.pkgName) g.reset() if err := g.genExampleFile(s); err != nil { return &g.resp, fmt.Errorf("error generating example for %q; %v", s.GetName(), err) } - 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.imports[pbinfo.ImportSpec{Name: g.cfg.pkgName, Path: g.cfg.pkgPath}] = true + g.commit(outFile+"_client_example_test.go", g.cfg.pkgName+"_test") g.reset() if err := g.genExampleIteratorFile(s); err != nil { return &g.resp, fmt.Errorf("error generating iter example for %q; %v", s.GetName(), err) } - g.imports[pbinfo.ImportSpec{Name: g.opts.pkgName, Path: g.opts.pkgPath}] = true - g.commitWithBuildTag(outFile+"_client_example_go123_test.go", g.opts.pkgName+"_test", "go1.23") + g.imports[pbinfo.ImportSpec{Name: g.cfg.pkgName, Path: g.cfg.pkgPath}] = true + g.commitWithBuildTag(outFile+"_client_example_go123_test.go", g.cfg.pkgName+"_test", "go1.23") // Replace original set of transports for the next service that may have // REST-able RPCs. if !hasREST { - g.opts.transports = transports + g.cfg.transports = transports } } if err := g.genAndCommitSnippetMetadata(protoPkg); err != nil { @@ -159,15 +158,15 @@ func gen(genReq *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse g.reset() g.genDocFile(time.Now().Year(), genServs) g.resp.File = append(g.resp.File, &pluginpb.CodeGeneratorResponse_File{ - Name: proto.String(filepath.Join(g.opts.outDir, "doc.go")), + Name: proto.String(filepath.Join(g.cfg.outDir, "doc.go")), Content: proto.String(g.pt.String()), }) - if g.opts.generateGAPICMetadata { + if g.cfg.generateGAPICMetadata { g.reset() g.genGapicMetadataFile() g.resp.File = append(g.resp.File, &pluginpb.CodeGeneratorResponse_File{ - Name: proto.String(filepath.Join(g.opts.outDir, "gapic_metadata.json")), + Name: proto.String(filepath.Join(g.cfg.outDir, "gapic_metadata.json")), Content: proto.String(g.pt.String()), }) } @@ -177,7 +176,7 @@ func gen(genReq *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse if err := g.customOperationType(); err != nil { return nil, err } - g.commit(filepath.Join(g.opts.outDir, "operations.go"), g.opts.pkgName) + g.commit(filepath.Join(g.cfg.outDir, "operations.go"), g.cfg.pkgName) } g.reset() @@ -235,7 +234,7 @@ func (g *generator) genAndCommitHelpers(scopes []string) error { g.imports[pbinfo.ImportSpec{Path: "google.golang.org/api/option"}] = true g.imports[pbinfo.ImportSpec{Path: "google.golang.org/protobuf/runtime/protoimpl"}] = true - p("const serviceName = %q", g.serviceConfig.GetName()) + p("const serviceName = %q", g.cfg.APIServiceConfig.GetName()) // Note: protoimpl currently only exposes their minor version. If they ever // take a v2 we will need to update this code accordingly and/or ask them to // expose the major version as well. @@ -267,7 +266,7 @@ func (g *generator) genAndCommitHelpers(scopes []string) error { p(" }") p("}") p("") - if containsTransport(g.opts.transports, rest) { + if containsTransport(g.cfg.transports, rest) { g.imports[pbinfo.ImportSpec{Path: "io"}] = true g.imports[pbinfo.ImportSpec{Path: "log/slog"}] = true g.imports[pbinfo.ImportSpec{Path: "net/http"}] = true @@ -314,7 +313,7 @@ func (g *generator) genAndCommitHelpers(scopes []string) error { p("") } - if containsTransport(g.opts.transports, grpc) { + if containsTransport(g.cfg.transports, grpc) { g.imports[pbinfo.ImportSpec{Path: "log/slog"}] = true g.imports[pbinfo.ImportSpec{Path: "github.com/googleapis/gax-go/v2/internallog/grpclog"}] = true g.imports[pbinfo.ImportSpec{Path: "google.golang.org/grpc"}] = true @@ -333,8 +332,8 @@ func (g *generator) genAndCommitHelpers(scopes []string) error { p("") } - outFile := filepath.Join(g.opts.outDir, "helpers.go") - g.commit(outFile, g.opts.pkgName) + outFile := filepath.Join(g.cfg.outDir, "helpers.go") + g.commit(outFile, g.cfg.pkgName) return nil } @@ -342,7 +341,7 @@ func (g *generator) genAndCommitHelpers(scopes []string) error { func (g *generator) gen(serv *descriptorpb.ServiceDescriptorProto) error { // If using service name overrides, use that directly for the rest of generation. override := g.getServiceNameOverride(serv) - servName := pbinfo.ReduceServNameWithOverride(serv.GetName(), g.opts.pkgName, override) + servName := pbinfo.ReduceServNameWithOverride(serv.GetName(), g.cfg.pkgName, override) g.clientHook(servName) if err := g.clientOptions(serv, servName); err != nil { @@ -352,7 +351,7 @@ func (g *generator) gen(serv *descriptorpb.ServiceDescriptorProto) error { return err } - for _, v := range g.opts.transports { + for _, v := range g.cfg.transports { switch v { case grpc: if err := g.genGRPCMethods(serv, servName); err != nil { @@ -655,7 +654,7 @@ func (g *generator) methodDoc(m *descriptorpb.MethodDescriptorProto, serv *descr return } - if containsTransport(g.opts.transports, rest) && m.GetClientStreaming() { + if containsTransport(g.cfg.transports, rest) && m.GetClientStreaming() { com = fmt.Sprintf("%s\n\nThis method is not supported for the REST transport.", com) } // If the method is marked as deprecated and there is no comment, then add default deprecation comment. diff --git a/internal/gengapic/gengapic_test.go b/internal/gengapic/gengapic_test.go index a5c3b0d9237..0bc148a1061 100644 --- a/internal/gengapic/gengapic_test.go +++ b/internal/gengapic/gengapic_test.go @@ -326,8 +326,27 @@ func TestGenGRPCMethods(t *testing.T) { } var g generator - g.opts = &options{ + g.cfg = &generatorConfig{ pkgName: "pkg", + APIServiceConfig: &serviceconfig.Service{ + Publishing: &annotations.Publishing{ + MethodSettings: []*annotations.MethodSettings{ + { + Selector: "my.pkg.Foo.GetEmptyThing", + AutoPopulatedFields: []string{ + "request_id", + }, + }, + { + Selector: "my.pkg.Foo.GetOneThing", + AutoPopulatedFields: []string{ + "request_id", + "non_proto3optional_request_id", + }, + }, + }, + }, + }, } g.metadata = &metadatapb.GapicMetadata{ Services: make(map[string]*metadatapb.GapicMetadata_ServiceForTransport), @@ -339,26 +358,6 @@ func TestGenGRPCMethods(t *testing.T) { } g.imports = map[pbinfo.ImportSpec]bool{} - g.serviceConfig = &serviceconfig.Service{ - Publishing: &annotations.Publishing{ - MethodSettings: []*annotations.MethodSettings{ - { - Selector: "my.pkg.Foo.GetEmptyThing", - AutoPopulatedFields: []string{ - "request_id", - }, - }, - { - Selector: "my.pkg.Foo.GetOneThing", - AutoPopulatedFields: []string{ - "request_id", - "non_proto3optional_request_id", - }, - }, - }, - }, - } - cpb := &conf.ServiceConfig{ MethodConfig: []*conf.MethodConfig{ { @@ -376,7 +375,7 @@ func TestGenGRPCMethods(t *testing.T) { t.Error(err) } in := bytes.NewReader(data) - g.grpcConf, err = conf.New(in) + g.cfg.gRPCServiceConfig, err = conf.New(in) if err != nil { t.Error(err) } @@ -606,7 +605,7 @@ func TestMethodDoc(t *testing.T) { for _, tst := range []struct { in, want string clientStreaming, deprecated bool - opts options + cfg *generatorConfig }{ { in: "", @@ -615,35 +614,40 @@ func TestMethodDoc(t *testing.T) { { in: "Does stuff.\nIt also does other stuffs.", want: "// MyMethod does stuff.\n// It also does other stuffs.\n", + cfg: &generatorConfig{}, }, { in: "This is deprecated.\nIt does not have a proper comment.", want: "// MyMethod this is deprecated.\n// It does not have a proper comment.\n//\n// Deprecated: MyMethod may be removed in a future version.\n", deprecated: true, + cfg: &generatorConfig{}, }, { in: "Deprecated: this is a proper deprecation notice.", want: "// MyMethod is deprecated.\n//\n// Deprecated: this is a proper deprecation notice.\n", deprecated: true, + cfg: &generatorConfig{}, }, { in: "Does my thing.\nDeprecated: this is a proper deprecation notice.", want: "// MyMethod does my thing.\n// Deprecated: this is a proper deprecation notice.\n", deprecated: true, + cfg: &generatorConfig{}, }, { in: "", want: "// MyMethod is deprecated.\n//\n// Deprecated: MyMethod may be removed in a future version.\n", deprecated: true, + cfg: &generatorConfig{}, }, { in: "Does client streaming stuff.\nIt also does other stuffs.", want: "// MyMethod does client streaming stuff.\n// It also does other stuffs.\n//\n// This method is not supported for the REST transport.\n", clientStreaming: true, - opts: options{transports: []transport{rest}}, + cfg: &generatorConfig{transports: []transport{rest}}, }, } { - g.opts = &tst.opts + g.cfg = tst.cfg sm := snippets.NewMetadata("mypackage", "github.com/googleapis/mypackage", "mypackagego") sm.AddService(servName, "mypackage.googleapis.com") sm.AddMethod(servName, methodName, "mypackage", servName, 50) @@ -713,27 +717,27 @@ func TestGenOperationBuilders(t *testing.T) { var g generator g.imports = map[pbinfo.ImportSpec]bool{} - g.serviceConfig = &serviceconfig.Service{ - Publishing: &annotations.Publishing{ - MethodSettings: []*annotations.MethodSettings{ - { - // Enable auto-populated field request_id only for EmptyLRO, not RespLRO. - Selector: "my.pkg.Foo.EmptyLRO", - AutoPopulatedFields: []string{ - "request_id", - }, - }, - }, - }, - } g.mixins = mixins{ "google.longrunning.Operations": operationsMethods(), "google.cloud.location.Locations": locationMethods(), "google.iam.v1.IAMPolicy": iamPolicyMethods(), } - g.opts = &options{ + g.cfg = &generatorConfig{ pkgName: "pkg", transports: []transport{grpc}, + APIServiceConfig: &serviceconfig.Service{ + Publishing: &annotations.Publishing{ + MethodSettings: []*annotations.MethodSettings{ + { + // Enable auto-populated field request_id only for EmptyLRO, not RespLRO. + Selector: "my.pkg.Foo.EmptyLRO", + AutoPopulatedFields: []string{ + "request_id", + }, + }, + }, + }, + }, } cpb := &conf.ServiceConfig{ MethodConfig: []*conf.MethodConfig{ @@ -752,7 +756,7 @@ func TestGenOperationBuilders(t *testing.T) { t.Error(err) } in := bytes.NewReader(data) - g.grpcConf, err = conf.New(in) + g.cfg.gRPCServiceConfig, err = conf.New(in) if err != nil { t.Error(err) } @@ -1233,7 +1237,7 @@ func TestReturnType(t *testing.T) { message: op, }, }, - opts: &options{ + cfg: &generatorConfig{ generateAsDIREGAPIC: true, }, descInfo: pbinfo.Info{ @@ -1336,7 +1340,7 @@ func TestCollectServicesAndScopes(t *testing.T) { want: []*descriptorpb.ServiceDescriptorProto{mixinFiles["google.iam.v1.IAMPolicy"][0].GetService()[0]}, }, } { - g := &generator{opts: &options{pkgPath: tst.goPkgPath}} + g := &generator{cfg: &generatorConfig{pkgPath: tst.goPkgPath}} setOAuthScopes(libraryServ, tst.wantScopes) gotServices, gotScopes := g.collectServicesAndScopes(&pluginpb.CodeGeneratorRequest{ FileToGenerate: tst.toGen, @@ -1353,13 +1357,13 @@ func TestCollectServicesAndScopes(t *testing.T) { func TestGenAndCommentHelpers(t *testing.T) { g := generator{ - apiName: sample.ServiceTitle, - imports: map[pbinfo.ImportSpec]bool{}, - serviceConfig: sample.ServiceConfig(), - opts: &options{ - pkgPath: sample.GoPackagePath, - pkgName: sample.GoPackageName, - transports: []transport{grpc, rest}, + apiName: sample.ServiceTitle, + imports: map[pbinfo.ImportSpec]bool{}, + cfg: &generatorConfig{ + pkgPath: sample.GoPackagePath, + pkgName: sample.GoPackageName, + transports: []transport{grpc, rest}, + APIServiceConfig: sample.ServiceConfig(), }, } inputType := sample.InputType(sample.CreateRequest) diff --git a/internal/gengapic/gengrpc.go b/internal/gengapic/gengrpc.go index f0b2b7d9ea3..f09fbcad781 100644 --- a/internal/gengapic/gengrpc.go +++ b/internal/gengapic/gengrpc.go @@ -162,7 +162,7 @@ func (g *generator) emptyUnaryGRPCCall(servName string, m *descriptorpb.MethodDe func (g *generator) grpcStubCall(method *descriptorpb.MethodDescriptorProto) string { service := g.descInfo.ParentElement[method].(*descriptorpb.ServiceDescriptorProto) override := g.getServiceNameOverride(service) - stub := pbinfo.ReduceServNameWithOverride(service.GetName(), g.opts.pkgName, override) + stub := pbinfo.ReduceServNameWithOverride(service.GetName(), g.cfg.pkgName, override) return fmt.Sprintf("executeRPC(ctx, c.%s.%s, req, settings.GRPC, c.logger, %q)", grpcClientField(stub), method.GetName(), method.GetName()) } @@ -186,7 +186,7 @@ func (g *generator) grpcClientOptions(serv *descriptorpb.ServiceDescriptorProto, p(" internaloption.WithDefaultAudience(%q),", generateDefaultAudience(host)) p(" internaloption.WithDefaultScopes(DefaultAuthScopes()...),") p(" internaloption.EnableJwtWithScope(),") - if _, ok := enableMtlsHardBoundTokens[g.serviceConfig.GetName()]; ok { + if _, ok := enableMtlsHardBoundTokens[g.cfg.APIServiceConfig.GetName()]; ok { p("internaloption.AllowHardBoundTokens(\"MTLS_S2A\"),") } p(" internaloption.EnableNewAuthLibrary(),") @@ -207,7 +207,7 @@ func (g *generator) grpcCallOptions(serv *descriptorpb.ServiceDescriptorProto, s p := g.printf // defaultCallOptions - c := g.grpcConf + c := g.cfg.gRPCServiceConfig methods := append(serv.GetMethod(), g.getMixinMethods()...) diff --git a/internal/gengapic/gengrpc_test.go b/internal/gengapic/gengrpc_test.go index b9498c5862e..419912f2dac 100644 --- a/internal/gengapic/gengrpc_test.go +++ b/internal/gengapic/gengrpc_test.go @@ -53,9 +53,20 @@ func TestServiceRenaming(t *testing.T) { var g generator g.imports = map[pbinfo.ImportSpec]bool{} - g.opts = &options{ + g.cfg = &generatorConfig{ pkgName: "pkg", transports: []transport{grpc}, + APIServiceConfig: &serviceconfig.Service{ + Publishing: &annotations.Publishing{ + LibrarySettings: []*annotations.ClientLibrarySettings{ + { + GoSettings: &annotations.GoSettings{ + RenamedServices: map[string]string{"Foo": "Bar"}, + }, + }, + }, + }, + }, } cpb := &conf.ServiceConfig{ MethodConfig: []*conf.MethodConfig{ @@ -74,7 +85,7 @@ func TestServiceRenaming(t *testing.T) { t.Error(err) } in := bytes.NewReader(data) - g.grpcConf, err = conf.New(in) + g.cfg.gRPCServiceConfig, err = conf.New(in) if err != nil { t.Error(err) } @@ -88,17 +99,6 @@ func TestServiceRenaming(t *testing.T) { } g.descInfo.ParentFile[serv] = file g.descInfo.ParentElement = map[pbinfo.ProtoType]pbinfo.ProtoType{} - g.serviceConfig = &serviceconfig.Service{ - Publishing: &annotations.Publishing{ - LibrarySettings: []*annotations.ClientLibrarySettings{ - { - GoSettings: &annotations.GoSettings{ - RenamedServices: map[string]string{"Foo": "Bar"}, - }, - }, - }, - }, - } m := &descriptorpb.MethodDescriptorProto{ Name: proto.String("Baz"), diff --git a/internal/gengapic/genrest.go b/internal/gengapic/genrest.go index d0308187331..393d5a1b6d2 100644 --- a/internal/gengapic/genrest.go +++ b/internal/gengapic/genrest.go @@ -81,7 +81,7 @@ func (g *generator) restClientInit(serv *descriptorpb.ServiceDescriptorProto, se g.imports[pbinfo.ImportSpec{Name: "lroauto", Path: "cloud.google.com/go/longrunning/autogen"}] = true } if opServ, ok := g.customOpServices[serv]; ok { - opServName := pbinfo.ReduceServName(opServ.GetName(), g.opts.pkgName) + opServName := pbinfo.ReduceServName(opServ.GetName(), g.cfg.pkgName) p("// operationClient is used to call the operation-specific management service.") p("operationClient *%sClient", opServName) p("") @@ -189,7 +189,7 @@ func (g *generator) restClientUtilities(serv *descriptorpb.ServiceDescriptorProt p("") } if hasCustomOp { - opServName := pbinfo.ReduceServName(opServ.GetName(), g.opts.pkgName) + opServName := pbinfo.ReduceServName(opServ.GetName(), g.cfg.pkgName) p("o := []option.ClientOption{") p(" option.WithHTTPClient(httpClient),") p(" option.WithEndpoint(endpoint),") @@ -405,11 +405,11 @@ func (g *generator) generateQueryString(m *descriptorpb.MethodDescriptorProto) { } sort.Strings(fields) - if g.opts.restNumericEnum || len(fields) > 0 { + if g.cfg.restNumericEnum || len(fields) > 0 { g.imports[pbinfo.ImportSpec{Path: "net/url"}] = true p("params := url.Values{}") } - if g.opts.restNumericEnum { + if g.cfg.restNumericEnum { p(`params.Add("$alt", "json;enum-encoding=int")`) } for _, path := range fields { @@ -492,7 +492,7 @@ func (g *generator) generateQueryString(m *descriptorpb.MethodDescriptorProto) { p("}") } - if g.opts.restNumericEnum || len(fields) > 0 { + if g.cfg.restNumericEnum || len(fields) > 0 { p("") p("baseUrl.RawQuery = params.Encode()") p("") @@ -1189,7 +1189,7 @@ func (g *generator) protoJSONMarshaler() { // enum. However, since they are synthetic protos, the enum numbers likely // do not match the internal proto enum value, so we do not want to send // any enums as numbers with DIREGAPIC clients. - if g.opts.generateAsDIREGAPIC { + if g.cfg.generateAsDIREGAPIC { marshalOpts = "AllowPartial: true" } g.pt.Printf("m := protojson.MarshalOptions{%s}", marshalOpts) @@ -1199,7 +1199,7 @@ func (g *generator) restCallOptions(serv *descriptorpb.ServiceDescriptorProto, s p := g.printf // defaultCallOptions - c := g.grpcConf + c := g.cfg.gRPCServiceConfig methods := append(serv.GetMethod(), g.getMixinMethods()...) diff --git a/internal/gengapic/genrest_test.go b/internal/gengapic/genrest_test.go index e214badc967..38ed49c4ccb 100644 --- a/internal/gengapic/genrest_test.go +++ b/internal/gengapic/genrest_test.go @@ -696,7 +696,7 @@ func TestGenRestMethod(t *testing.T) { methodToWrapper: map[*descriptorpb.MethodDescriptorProto]operationWrapper{}, opWrappers: map[string]operationWrapper{}, }, - opts: &options{}, + cfg: &generatorConfig{}, customOpServices: map[*descriptorpb.ServiceDescriptorProto]*descriptorpb.ServiceDescriptorProto{ s: opS, }, @@ -747,13 +747,13 @@ func TestGenRestMethod(t *testing.T) { for _, tst := range []struct { name string method *descriptorpb.MethodDescriptorProto - options *options + cfg *generatorConfig imports map[pbinfo.ImportSpec]bool }{ { - name: "custom_op", - method: opRPC, - options: &options{generateAsDIREGAPIC: true}, + name: "custom_op", + method: opRPC, + cfg: &generatorConfig{generateAsDIREGAPIC: true}, imports: map[pbinfo.ImportSpec]bool{ {Path: "google.golang.org/protobuf/encoding/protojson"}: true, {Path: "net/url"}: true, @@ -762,9 +762,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "empty_rpc", - method: emptyRPC, - options: &options{}, + name: "empty_rpc", + method: emptyRPC, + cfg: &generatorConfig{}, imports: map[pbinfo.ImportSpec]bool{ {Path: "fmt"}: true, {Path: "github.com/google/uuid"}: true, @@ -773,9 +773,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "unary_rpc", - method: unaryRPC, - options: &options{restNumericEnum: true}, + name: "unary_rpc", + method: unaryRPC, + cfg: &generatorConfig{restNumericEnum: true}, imports: map[pbinfo.ImportSpec]bool{ {Path: "bytes"}: true, {Path: "fmt"}: true, @@ -788,9 +788,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "paging_rpc", - method: pagingRPC, - options: &options{}, + name: "paging_rpc", + method: pagingRPC, + cfg: &generatorConfig{}, imports: map[pbinfo.ImportSpec]bool{ {Path: "math"}: true, {Path: "net/url"}: true, @@ -802,9 +802,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "server_stream_rpc", - method: serverStreamRPC, - options: &options{}, + name: "server_stream_rpc", + method: serverStreamRPC, + cfg: &generatorConfig{}, imports: map[pbinfo.ImportSpec]bool{ {Path: "bytes"}: true, {Path: "context"}: true, @@ -819,9 +819,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "no_request_stream_rpc", - method: clientStreamRPC, - options: &options{}, + name: "no_request_stream_rpc", + method: clientStreamRPC, + cfg: &generatorConfig{}, imports: map[pbinfo.ImportSpec]bool{ {Path: "context"}: true, {Path: "errors"}: true, @@ -829,9 +829,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "lro_rpc", - method: lroRPC, - options: &options{transports: []transport{rest}}, + name: "lro_rpc", + method: lroRPC, + cfg: &generatorConfig{transports: []transport{rest}}, imports: map[pbinfo.ImportSpec]bool{ {Path: "bytes"}: true, {Path: "cloud.google.com/go/longrunning"}: true, @@ -843,9 +843,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "http_body_rpc", - method: httpBodyRPC, - options: &options{}, + name: "http_body_rpc", + method: httpBodyRPC, + cfg: &generatorConfig{}, imports: map[pbinfo.ImportSpec]bool{ {Path: "bytes"}: true, {Path: "fmt"}: true, @@ -858,9 +858,9 @@ func TestGenRestMethod(t *testing.T) { }, }, { - name: "update_rpc", - method: updateRPC, - options: &options{restNumericEnum: true}, + name: "update_rpc", + method: updateRPC, + cfg: &generatorConfig{restNumericEnum: true}, imports: map[pbinfo.ImportSpec]bool{ {Path: "bytes"}: true, {Path: "fmt"}: true, @@ -872,9 +872,9 @@ func TestGenRestMethod(t *testing.T) { } { t.Run(fmt.Sprintf("%s_%s", t.Name(), tst.name), func(t *testing.T) { s.Method = []*descriptorpb.MethodDescriptorProto{tst.method} - g.opts = tst.options + g.cfg = tst.cfg g.imports = make(map[pbinfo.ImportSpec]bool) - g.serviceConfig = &serviceconfig.Service{ + g.cfg.APIServiceConfig = &serviceconfig.Service{ Http: &annotations.Http{ Rules: []*annotations.HttpRule{ { diff --git a/internal/gengapic/lro.go b/internal/gengapic/lro.go index b52a1f47fcb..1c93d56bcf5 100644 --- a/internal/gengapic/lro.go +++ b/internal/gengapic/lro.go @@ -98,7 +98,7 @@ func (g *generator) genOperationBuilder(servName string, m *descriptorpb.MethodD // LRO from name { - for _, t := range g.opts.transports { + for _, t := range g.cfg.transports { p("// %[1]s returns a new %[1]s from a given name.", ow.name) p("// The name must be that of a previously created %s, possibly from a different process.", ow.name) diff --git a/internal/gengapic/mixins.go b/internal/gengapic/mixins.go index 4a835e9121e..b28af162fac 100644 --- a/internal/gengapic/mixins.go +++ b/internal/gengapic/mixins.go @@ -59,7 +59,7 @@ func initMixinFiles() { // collectMixins collects the configured mixin APIs from the Service config and // gathers the appropriately configured mixin methods to generate for each. func (g *generator) collectMixins() { - for _, api := range g.serviceConfig.GetApis() { + for _, api := range g.cfg.APIServiceConfig.GetApis() { if _, ok := mixinFiles[api.GetName()]; ok { g.mixins[api.GetName()] = g.collectMixinMethods(api.GetName()) } @@ -92,7 +92,7 @@ func (g *generator) collectMixinMethods(api string) []*descriptorpb.MethodDescri } // Overwrite the google.api.http annotations with bindings from the Service config. - for _, rule := range g.serviceConfig.GetHttp().GetRules() { + for _, rule := range g.cfg.APIServiceConfig.GetHttp().GetRules() { m, match := methods[rule.GetSelector()] if !match { continue @@ -103,7 +103,7 @@ func (g *generator) collectMixinMethods(api string) []*descriptorpb.MethodDescri } // Include any documentation from the Service config. - for _, rule := range g.serviceConfig.GetDocumentation().GetRules() { + for _, rule := range g.cfg.APIServiceConfig.GetDocumentation().GetRules() { m, match := methods[rule.GetSelector()] if !match { continue @@ -191,19 +191,19 @@ func (g *generator) mixinStubsInit() { // hasLROMixin is a convenience method for determining if the Operations mixin // should be generated. func (g *generator) hasLROMixin() bool { - return len(g.mixins["google.longrunning.Operations"]) > 0 && len(g.serviceConfig.GetApis()) > 1 + return len(g.mixins["google.longrunning.Operations"]) > 0 && len(g.cfg.APIServiceConfig.GetApis()) > 1 } // hasIAMPolicyMixin is a convenience method for determining if the IAMPolicy // mixin should be generated. func (g *generator) hasIAMPolicyMixin() bool { - return len(g.mixins["google.iam.v1.IAMPolicy"]) > 0 && !g.hasIAMPolicyOverrides && len(g.serviceConfig.GetApis()) > 1 + return len(g.mixins["google.iam.v1.IAMPolicy"]) > 0 && !g.hasIAMPolicyOverrides && len(g.cfg.APIServiceConfig.GetApis()) > 1 } // hasLocationMixin is a convenience method for determining if the Locations // mixin should be generated. func (g *generator) hasLocationMixin() bool { - return len(g.mixins["google.cloud.location.Locations"]) > 0 && len(g.serviceConfig.GetApis()) > 1 + return len(g.mixins["google.cloud.location.Locations"]) > 0 && len(g.cfg.APIServiceConfig.GetApis()) > 1 } // containsIAMPolicyOverrides determines if any of the given services define an @@ -231,13 +231,13 @@ func (g *generator) containsIAMPolicyOverrides(servs []*descriptorpb.ServiceDesc // protos-to-be-generated file set based on if the Go package to be generated // is for one of the mixin services explicitly or not. func (g *generator) includeMixinInputFile(file string) bool { - if strings.HasPrefix(file, "google/cloud/location") && !strings.Contains(g.opts.pkgPath, "location") { + if strings.HasPrefix(file, "google/cloud/location") && !strings.Contains(g.cfg.pkgPath, "location") { return false } - if strings.HasPrefix(file, "google/iam/v1") && !strings.Contains(g.opts.pkgPath, "iam") { + if strings.HasPrefix(file, "google/iam/v1") && !strings.Contains(g.cfg.pkgPath, "iam") { return false } - if strings.HasPrefix(file, "google/longrunning") && !strings.Contains(g.opts.pkgPath, "longrunning") { + if strings.HasPrefix(file, "google/longrunning") && !strings.Contains(g.cfg.pkgPath, "longrunning") { return false } // Not a mixin file or generating a mixin GAPIC explicitly so include the file. @@ -247,7 +247,7 @@ func (g *generator) includeMixinInputFile(file string) bool { // lookUpGetOperationOverride looks up the google.api.http rule defined in the // service config for the given RPC. func (g *generator) lookupHTTPOverride(fqn string, f func(h *annotations.HttpRule) string) string { - for _, rule := range g.serviceConfig.GetHttp().GetRules() { + for _, rule := range g.cfg.APIServiceConfig.GetHttp().GetRules() { if rule.GetSelector() == fqn { return f(rule) } diff --git a/internal/gengapic/mixins_test.go b/internal/gengapic/mixins_test.go index d49ca531a94..37cb176fa5e 100644 --- a/internal/gengapic/mixins_test.go +++ b/internal/gengapic/mixins_test.go @@ -49,25 +49,27 @@ func TestCollectMixins(t *testing.T) { g := generator{ comments: make(map[protoiface.MessageV1]string), mixins: make(mixins), - serviceConfig: &serviceconfig.Service{ - Apis: []*apipb.Api{ - {Name: "google.example.library.v1.Library"}, - {Name: "google.longrunning.Operations"}, - {Name: "google.cloud.location.Locations"}, - {Name: "google.iam.v1.IAMPolicy"}, - }, - Http: &annotations.Http{ - Rules: []*annotations.HttpRule{ - operationsHTTP, - locationHTTP, - iamHTTP, + cfg: &generatorConfig{ + APIServiceConfig: &serviceconfig.Service{ + Apis: []*apipb.Api{ + {Name: "google.example.library.v1.Library"}, + {Name: "google.longrunning.Operations"}, + {Name: "google.cloud.location.Locations"}, + {Name: "google.iam.v1.IAMPolicy"}, }, - }, - Documentation: &serviceconfig.Documentation{ - Rules: []*serviceconfig.DocumentationRule{ - { - Selector: "google.iam.v1.IAMPolicy.GetIamPolicy", - Description: iamDescription, + Http: &annotations.Http{ + Rules: []*annotations.HttpRule{ + operationsHTTP, + locationHTTP, + iamHTTP, + }, + }, + Documentation: &serviceconfig.Documentation{ + Rules: []*serviceconfig.DocumentationRule{ + { + Selector: "google.iam.v1.IAMPolicy.GetIamPolicy", + Description: iamDescription, + }, }, }, }, @@ -132,10 +134,12 @@ func TestHasIAMPolicyMixin(t *testing.T) { "google.longrunning.Operations": operationsMethods(), "google.cloud.location.Locations": locationMethods(), }, - serviceConfig: &serviceconfig.Service{ - Apis: []*apipb.Api{ - {Name: "foo.bar.Baz"}, - {Name: "google.iam.v1.IAMPolicy"}, + cfg: &generatorConfig{ + APIServiceConfig: &serviceconfig.Service{ + Apis: []*apipb.Api{ + {Name: "foo.bar.Baz"}, + {Name: "google.iam.v1.IAMPolicy"}, + }, }, }, } @@ -152,13 +156,13 @@ func TestHasIAMPolicyMixin(t *testing.T) { } want = false - g.serviceConfig.Apis = []*apipb.Api{{Name: "google.iam.v1.IAMPolicy"}} + g.cfg.APIServiceConfig.Apis = []*apipb.Api{{Name: "google.iam.v1.IAMPolicy"}} if got := g.hasIAMPolicyMixin(); !cmp.Equal(got, want) { t.Errorf("TestHasIAMPolicyMixin wanted %v but got %v", want, got) } g.hasIAMPolicyOverrides = true - g.serviceConfig.Apis = append(g.serviceConfig.Apis, &apipb.Api{Name: "foo.bar.Baz"}) + g.cfg.APIServiceConfig.Apis = append(g.cfg.APIServiceConfig.Apis, &apipb.Api{Name: "foo.bar.Baz"}) if got := g.hasIAMPolicyMixin(); !cmp.Equal(got, want) { t.Errorf("TestHasIAMPolicyMixin wanted %v but got %v", want, got) } @@ -198,10 +202,12 @@ func TestHasLocationMixin(t *testing.T) { "google.longrunning.Operations": operationsMethods(), "google.iam.v1.IAMPolicy": iamPolicyMethods(), }, - serviceConfig: &serviceconfig.Service{ - Apis: []*apipb.Api{ - {Name: "foo.bar.Baz"}, - {Name: "google.cloud.location.Locations"}, + cfg: &generatorConfig{ + APIServiceConfig: &serviceconfig.Service{ + Apis: []*apipb.Api{ + {Name: "foo.bar.Baz"}, + {Name: "google.cloud.location.Locations"}, + }, }, }, } @@ -218,7 +224,7 @@ func TestHasLocationMixin(t *testing.T) { } want = false - g.serviceConfig.Apis = []*apipb.Api{{Name: "google.cloud.location.Locations"}} + g.cfg.APIServiceConfig.Apis = []*apipb.Api{{Name: "google.cloud.location.Locations"}} if got := g.hasLocationMixin(); !cmp.Equal(got, want) { t.Errorf("TestHasLocationMixin wanted %v but got %v", want, got) } @@ -230,11 +236,13 @@ func TestHasLROMixin(t *testing.T) { "google.cloud.location.Locations": locationMethods(), "google.iam.v1.IAMPolicy": iamPolicyMethods(), }, - serviceConfig: &serviceconfig.Service{ - Apis: []*apipb.Api{ - {Name: "foo.bar.Baz"}, - {Name: "google.iam.v1.IAMPolicy"}, - {Name: "google.cloud.location.Locations"}, + cfg: &generatorConfig{ + APIServiceConfig: &serviceconfig.Service{ + Apis: []*apipb.Api{ + {Name: "foo.bar.Baz"}, + {Name: "google.iam.v1.IAMPolicy"}, + {Name: "google.cloud.location.Locations"}, + }, }, }, } @@ -251,7 +259,7 @@ func TestHasLROMixin(t *testing.T) { } want = false - g.serviceConfig.Apis = []*apipb.Api{{Name: "google.longrunning.Operations"}} + g.cfg.APIServiceConfig.Apis = []*apipb.Api{{Name: "google.longrunning.Operations"}} if got := g.hasLROMixin(); !cmp.Equal(got, want) { t.Errorf("TestHasLROMixin wanted %v but got %v", want, got) } @@ -293,8 +301,10 @@ func TestGetOperationPathOverride(t *testing.T) { g := generator{ comments: make(map[protoiface.MessageV1]string), mixins: make(mixins), - serviceConfig: &serviceconfig.Service{ - Http: tc.http, + cfg: &generatorConfig{ + APIServiceConfig: &serviceconfig.Service{ + Http: tc.http, + }, }, } if got := g.getOperationPathOverride(tc.pkg); !cmp.Equal(got, tc.want) { @@ -351,7 +361,7 @@ func TestIncludeMixinInputFile(t *testing.T) { want: true, }, } { - g := &generator{opts: &options{pkgPath: tst.pkgPath}} + g := &generator{cfg: &generatorConfig{pkgPath: tst.pkgPath}} if got := g.includeMixinInputFile(tst.file); got != tst.want { t.Errorf("%s: got %v, want %v", tst.name, got, tst.want) } diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index e7ed6de0622..d2a95267d89 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -17,9 +17,15 @@ package gengapic import ( "errors" "fmt" + "os" "path/filepath" "sort" "strings" + + "github.com/ghodss/yaml" + conf "github.com/googleapis/gapic-generator-go/internal/grpc_service_config" + "google.golang.org/genproto/googleapis/api/serviceconfig" + "google.golang.org/protobuf/encoding/protojson" ) // Define a type to represent supported network transports. @@ -72,8 +78,7 @@ var SupportedPrefixArgs map[string]func(string) configOption = map[string]func(s // Configuration needed to drive the operation of the plugin. // The options should be treated as immutable once instantiated. -// TODO: rename this in a subsequent refactor (generatorConfig) -type options struct { +type generatorConfig struct { // The fully qualified package path, e.g. "cloud.google.com/go/foo/v1/foopb" pkgPath string // The package name, e.g. "foopb" @@ -97,13 +102,11 @@ type options struct { // TODO: rename this in a subsequent refactor omitSnippets bool - // path to the API service config - // TODO: rename and handle parsing during configuration - serviceConfigPath string + // Parsed Service Configuration. + APIServiceConfig *serviceconfig.Service - // path to the GRPC service config (e.g. method retry settings) - // TODO: rename and handle parsing during configuration - grpcConfPath string + // Parsed gRPC Service Configuration. + gRPCServiceConfig conf.Config // prefix for the enclosing module modulePrefix string @@ -123,7 +126,7 @@ type options struct { // Signature for configuration arguments. // Errors are part of the signature to allow options to provide validation feedback early. // Config options that return errors should not modify the configuration. -type configOption func(*options) error +type configOption func(*generatorConfig) error // NewOptionsFromParams consumes the "parameter" field from the CodeGenerationRequest to produce a configuration. // This should be a comma seperated list of plugin arguments, and each one is handled in the order it appears. @@ -134,12 +137,12 @@ type configOption func(*options) error // SupportedBooleanArgs // SupportedValueArgs // SupportedPrefixArgs -func newOptionsFromParams(generationParameter *string) (*options, error) { +func configFromRequest(generationParameter *string) (*generatorConfig, error) { if generationParameter == nil { return nil, errors.New("generationParameter is nil, cannot configure") } - cfg := &options{} + cfg := &generatorConfig{} // params are comma seperated. Split and process each individually. for _, s := range strings.Split(*generationParameter, ",") { @@ -202,7 +205,7 @@ func newOptionsFromParams(generationParameter *string) (*options, error) { // // It also handles cases such as default values (e.g. what transports are enabled) where the configuration is more // than a simple boolean option. -func validateAndNormalizeOptions(cfg *options) error { +func validateAndNormalizeOptions(cfg *generatorConfig) error { // Normalize transports. If no transports are specified, generate gRPC only by default. if len(cfg.transports) == 0 { cfg.transports = []transport{grpc} @@ -237,11 +240,11 @@ func withGoGAPICPackage(s string) configOption { p := strings.IndexByte(s, ';') if p < 0 { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { return errInvalidPackageParam } } - return func(cfg *options) error { + return func(cfg *generatorConfig) error { cfg.pkgPath = s[0:p] cfg.pkgName = s[p+1:] cfg.outDir = filepath.FromSlash(cfg.pkgPath) @@ -251,7 +254,7 @@ func withGoGAPICPackage(s string) configOption { // generateGAPICMetadata enables generation of GAPIC metadata. func generateGAPICMetadata() configOption { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { cfg.generateGAPICMetadata = true return nil } @@ -262,7 +265,7 @@ func generateGAPICMetadata() configOption { // reverse-compiled from an API Discovery document, rather than directly from service // protos (e.g. compute). func generateAsDIREGAPIC() configOption { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { cfg.generateAsDIREGAPIC = true return nil } @@ -271,7 +274,7 @@ func generateAsDIREGAPIC() configOption { // enableRESTNumericEnums is a behavioral flag that causes the generated REST clients // to send the numeric value for enum fields rather than the string label. func enableRESTNumericEnums() configOption { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { cfg.restNumericEnum = true return nil } @@ -279,31 +282,71 @@ func enableRESTNumericEnums() configOption { // Should automatic generation of snippets be disabled. func enableOmitSnippets() configOption { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { cfg.omitSnippets = true return nil } } // Specifies the path to the API service config file. +// Option parses the path and does basic validation. func withAPIServiceConfigPath(s string) configOption { - return func(cfg *options) error { - cfg.serviceConfigPath = s + return func(cfg *generatorConfig) error { + + if s == "" { + return fmt.Errorf("provided API service config path was empty") + } + y, err := os.ReadFile(s) + if err != nil { + return fmt.Errorf("error reading API service config path (%q): %v", s, err) + } + + j, err := yaml.YAMLToJSON(y) + if err != nil { + return fmt.Errorf("error converting API service config from YAML to JSON: %v", err) + } + + sc := &serviceconfig.Service{} + if err := (protojson.UnmarshalOptions{DiscardUnknown: true}).Unmarshal(j, sc); err != nil { + fmt.Errorf("error unmarshaling api service config: %v", err) + } + + // An API Service Config will always have a `name` so if it is not populated, + // it's an invalid config. + if sc.GetName() == "" { + return fmt.Errorf("invalid API service config contents") + } + cfg.APIServiceConfig = sc return nil } } // Specifies the path to the gRPC service config file. func withGRPCServiceConfigPath(s string) configOption { - return func(cfg *options) error { - cfg.grpcConfPath = s + return func(cfg *generatorConfig) error { + + if s == "" { + return fmt.Errorf("provided gRPC service config path was empty") + } + + f, err := os.Open(s) + if err != nil { + return fmt.Errorf("error opening gRPC service config(%q): %v", s, err) + } + defer f.Close() + + sc, err := conf.New(f) + if err != nil { + return fmt.Errorf("error parsing gPRC service config: %v", err) + } + cfg.gRPCServiceConfig = sc return nil } } // Specifies the module prefix. func withModulePrefix(s string) configOption { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { cfg.modulePrefix = s return nil } @@ -311,7 +354,7 @@ func withModulePrefix(s string) configOption { // Specifies the release level of the generated artifacts. func withReleaseLevel(s string) configOption { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { // TODO: this should be validated against a well defined set of levels cfg.relLvl = s return nil @@ -323,17 +366,17 @@ func withReleaseLevel(s string) configOption { func withPackageOverride(s string) configOption { e := strings.IndexByte(s, '=') if e < 0 { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { return fmt.Errorf("invalid package override format, should be =: %q", s) } } key, val := s[:e], s[e+1:] if val == "" { - return func(cfg *options) error { + return func(cfg *generatorConfig) error { return fmt.Errorf("invalid plugin option value, missing value in key=value: %q", s) } } - return func(cfg *options) error { + return func(cfg *generatorConfig) error { if cfg.pkgOverrides == nil { cfg.pkgOverrides = make(map[string]string) } @@ -365,13 +408,13 @@ func withTransports(s string) configOption { case "rest": transports[rest] = true default: - return func(cfg *options) error { + return func(cfg *generatorConfig) error { return fmt.Errorf("invalid transport option: %q", t) } } } - return func(cfg *options) error { + return func(cfg *generatorConfig) error { for t := range transports { cfg.transports = append(cfg.transports, t) } diff --git a/internal/gengapic/options_test.go b/internal/gengapic/options_test.go index 7aab03345c4..ecab047d2c2 100644 --- a/internal/gengapic/options_test.go +++ b/internal/gengapic/options_test.go @@ -19,17 +19,18 @@ import ( "testing" "github.com/google/go-cmp/cmp" + conf "github.com/googleapis/gapic-generator-go/internal/grpc_service_config" ) func TestParseOptions(t *testing.T) { for _, tst := range []struct { - param string - expectedOpts *options - expectErr bool + param string + expectedCfg *generatorConfig + expectErr bool }{ { param: "transport=grpc,go-gapic-package=path;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{grpc}, pkgPath: "path", pkgName: "pkg", @@ -39,7 +40,7 @@ func TestParseOptions(t *testing.T) { }, { param: "transport=rest+grpc,go-gapic-package=path;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{grpc, rest}, pkgPath: "path", pkgName: "pkg", @@ -49,7 +50,7 @@ func TestParseOptions(t *testing.T) { }, { param: "go-gapic-package=path;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{grpc}, pkgPath: "path", pkgName: "pkg", @@ -58,7 +59,7 @@ func TestParseOptions(t *testing.T) { }, { param: "metadata,go-gapic-package=path;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{grpc}, pkgPath: "path", pkgName: "pkg", @@ -68,7 +69,7 @@ func TestParseOptions(t *testing.T) { }, { param: "module=path,go-gapic-package=path/to/out;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{grpc}, pkgPath: "path/to/out", pkgName: "pkg", @@ -79,7 +80,7 @@ func TestParseOptions(t *testing.T) { }, { param: "Mgoogle/example/library/v1/library.proto=new/import/path;pkg,go-gapic-package=path/to/out;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ pkgOverrides: map[string]string{"google/example/library/v1/library.proto": "new/import/path;pkg"}, transports: []transport{grpc}, pkgPath: "path/to/out", @@ -90,7 +91,7 @@ func TestParseOptions(t *testing.T) { }, { param: "transport=rest,rest-numeric-enums,go-gapic-package=path;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{rest}, pkgPath: "path", pkgName: "pkg", @@ -101,7 +102,7 @@ func TestParseOptions(t *testing.T) { }, { param: "omit-snippets,go-gapic-package=path;pkg", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{grpc}, pkgPath: "path", pkgName: "pkg", @@ -132,7 +133,7 @@ func TestParseOptions(t *testing.T) { { // Test empty parameter in the CSV. param: "go-gapic-package=path/to/imp;pkg,,module=path", - expectedOpts: &options{ + expectedCfg: &generatorConfig{ transports: []transport{grpc}, pkgPath: "path/to/imp", pkgName: "pkg", @@ -142,7 +143,7 @@ func TestParseOptions(t *testing.T) { expectErr: false, }, } { - opts, err := newOptionsFromParams(&tst.param) + opts, err := configFromRequest(&tst.param) if tst.expectErr && err == nil { t.Errorf("parseOptions(%s) expected error", tst.param) continue @@ -153,8 +154,8 @@ func TestParseOptions(t *testing.T) { continue } - if !reflect.DeepEqual(opts, tst.expectedOpts) { - t.Errorf("parseOptions(%s) = %+v, expected %+v", tst.param, opts, tst.expectedOpts) + if !reflect.DeepEqual(opts, tst.expectedCfg) { + t.Errorf("parseOptions(%s) = %+v, expected %+v", tst.param, opts, tst.expectedCfg) continue } } @@ -163,23 +164,23 @@ func TestParseOptions(t *testing.T) { func TestValidateAndNormalizeOptions(t *testing.T) { for _, tc := range []struct { description string - in *options + in *generatorConfig wantErr bool - want *options + want *generatorConfig }{ { description: "empty", - in: &options{}, + in: &generatorConfig{}, wantErr: true, // no package info }, { description: "minimal", - in: &options{ + in: &generatorConfig{ pkgPath: "path", pkgName: "name", outDir: "dir", }, - want: &options{ + want: &generatorConfig{ pkgPath: "path", pkgName: "name", outDir: "dir", @@ -188,7 +189,7 @@ func TestValidateAndNormalizeOptions(t *testing.T) { }, { description: "conflict diregapic enum", - in: &options{ + in: &generatorConfig{ pkgPath: "path", pkgName: "name", outDir: "dir", @@ -199,7 +200,7 @@ func TestValidateAndNormalizeOptions(t *testing.T) { }, { description: "mismatched module prefix", - in: &options{ + in: &generatorConfig{ pkgPath: "path", pkgName: "name", outDir: "foo/bar/baz", @@ -222,7 +223,7 @@ func TestValidateAndNormalizeOptions(t *testing.T) { t.Errorf("got unwanted err: %v", err) } } - if diff := cmp.Diff(got, tc.want, cmp.AllowUnexported(options{})); diff != "" { + if diff := cmp.Diff(got, tc.want, cmp.AllowUnexported(generatorConfig{}, conf.Config{})); diff != "" { t.Errorf("got(-), want(+):\n%s", diff) } }) diff --git a/internal/gengapic/paging_test.go b/internal/gengapic/paging_test.go index e9f0d955486..2f0965344e1 100644 --- a/internal/gengapic/paging_test.go +++ b/internal/gengapic/paging_test.go @@ -470,7 +470,7 @@ func TestPagingField(t *testing.T) { if err != nil { t.Fatal(err) } - g.opts = &options{transports: []transport{rest}} + g.cfg = &generatorConfig{transports: []transport{rest}} for _, tst := range []struct { mthd *descriptorpb.MethodDescriptorProto @@ -507,7 +507,7 @@ func TestPagingField(t *testing.T) { if err != nil { t.Fatal(err) } - g.opts = &options{transports: []transport{rest}} + g.cfg = &generatorConfig{transports: []transport{rest}} for _, tst := range []struct { mthd *descriptorpb.MethodDescriptorProto sizeField *descriptorpb.FieldDescriptorProto // A nil field means this is not a paged method diff --git a/internal/gengapic/snippets.go b/internal/gengapic/snippets.go index 4ea9e95c2d2..905ef925db4 100644 --- a/internal/gengapic/snippets.go +++ b/internal/gengapic/snippets.go @@ -30,16 +30,16 @@ import ( // newSnippetsMetadata initializes the model that will collect snippet metadata. // Does nothing and returns nil if opts.omitSnippets is true. func (g *generator) newSnippetsMetadata(protoPkg string) *snippets.SnippetMetadata { - if g.opts.omitSnippets { + if g.cfg.omitSnippets { return nil } - return snippets.NewMetadata(protoPkg, g.metadata.LibraryPackage, g.opts.pkgName) + return snippets.NewMetadata(protoPkg, g.metadata.LibraryPackage, g.cfg.pkgName) } // addSnippetsMetadataDoc sets the documentation for a method in the snippet metadata. // Does nothing and returns nil if opts.omitSnippets is true, or if the streaming type is not supported in example.go. func (g *generator) addSnippetsMetadataDoc(m *descriptorpb.MethodDescriptorProto, servName, doc string) { - if g.opts.omitSnippets || m.GetClientStreaming() != m.GetServerStreaming() { + if g.cfg.omitSnippets || m.GetClientStreaming() != m.GetServerStreaming() { // TODO(chrisdsmith): implement streaming examples correctly, see example.go TODO(pongad). return } @@ -49,7 +49,7 @@ func (g *generator) addSnippetsMetadataDoc(m *descriptorpb.MethodDescriptorProto // addSnippetsMetadataParams sets the parameters for a method in the snippet metadata. // Does nothing and returns nil if opts.omitSnippets is true, or if the streaming type is not supported in example.go. func (g *generator) addSnippetsMetadataParams(m *descriptorpb.MethodDescriptorProto, servName, requestType string) { - if g.opts.omitSnippets || m.GetClientStreaming() != m.GetServerStreaming() { + if g.cfg.omitSnippets || m.GetClientStreaming() != m.GetServerStreaming() { // TODO(chrisdsmith): implement streaming examples correctly, see example.go TODO(pongad). return } @@ -59,7 +59,7 @@ func (g *generator) addSnippetsMetadataParams(m *descriptorpb.MethodDescriptorPr // addSnippetsMetadataResult sets the result type for a method in the snippet metadata. // Does nothing and returns nil if opts.omitSnippets is true, or if the streaming type is not supported in example.go. func (g *generator) addSnippetsMetadataResult(m *descriptorpb.MethodDescriptorProto, servName, resultType string) { - if g.opts.omitSnippets || m.GetClientStreaming() != m.GetServerStreaming() { + if g.cfg.omitSnippets || m.GetClientStreaming() != m.GetServerStreaming() { // TODO(chrisdsmith): implement streaming examples correctly, see example.go TODO(pongad). return } @@ -69,7 +69,7 @@ func (g *generator) addSnippetsMetadataResult(m *descriptorpb.MethodDescriptorPr // genAndCommitSnippets generates and commits a snippet file for each method in a client. // Does nothing and returns nil if opts.omitSnippets is true. func (g *generator) genAndCommitSnippets(s *descriptorpb.ServiceDescriptorProto) error { - if g.opts.omitSnippets { + if g.cfg.omitSnippets { return nil } defaultHost := proto.GetExtension(s.Options, annotations.E_DefaultHost).(string) @@ -90,10 +90,10 @@ func (g *generator) genAndCommitSnippets(s *descriptorpb.ServiceDescriptorProto) if err := g.genSnippetFile(s, m); err != nil { return err } - g.imports[pbinfo.ImportSpec{Name: g.opts.pkgName, Path: g.opts.pkgPath}] = true + g.imports[pbinfo.ImportSpec{Name: g.cfg.pkgName, Path: g.cfg.pkgPath}] = true // Use the client short name in this filepath. // E.g. the client for LoggingServiceV2 is just "Client". - clientName := pbinfo.ReduceServName(servName, g.opts.pkgName) + "Client" + clientName := pbinfo.ReduceServName(servName, g.cfg.pkgName) + "Client" // Get the original proto namespace for the method (different from `s` only for mixins). f := g.descInfo.ParentFile[m] // Get the original proto service for the method (different from `s` only for mixins). @@ -112,7 +112,7 @@ func (g *generator) genSnippetFile(s *descriptorpb.ServiceDescriptorProto, m *de } regionTag := g.snippetMetadata.RegionTag(servName, m.GetName()) g.headerComment(fmt.Sprintf("[START %s]", regionTag)) - pkgName := g.opts.pkgName + pkgName := g.cfg.pkgName reducedServName := pbinfo.ReduceServName(servName, pkgName) @@ -130,7 +130,7 @@ func (g *generator) genSnippetFile(s *descriptorpb.ServiceDescriptorProto, m *de // genAndCommitSnippetMetadata generates and commits the snippet metadata to the generator response. // Does nothing and returns nil if opts.omitSnippets is true. func (g *generator) genAndCommitSnippetMetadata(protoPkg string) error { - if g.opts.omitSnippets { + if g.cfg.omitSnippets { return nil } g.reset() @@ -147,11 +147,11 @@ func (g *generator) genAndCommitSnippetMetadata(protoPkg string) error { } func (g *generator) snippetsOutDir() string { - if strings.Contains(g.opts.pkgPath, "cloud.google.com/go/") { + if strings.Contains(g.cfg.pkgPath, "cloud.google.com/go/") { // Write snippet metadata at the top level of the google-cloud-go namespace, not at the client package. // This matches the destination directory structure in google-cloud-go. - pkg := strings.TrimPrefix(g.opts.pkgPath, "cloud.google.com/go/") + pkg := strings.TrimPrefix(g.cfg.pkgPath, "cloud.google.com/go/") return filepath.Join("cloud.google.com/go", "internal", "generated", "snippets", filepath.FromSlash(pkg)) } - return filepath.Join(g.opts.outDir, "internal", "snippets") + return filepath.Join(g.cfg.outDir, "internal", "snippets") } diff --git a/internal/gengapic/snippets_test.go b/internal/gengapic/snippets_test.go index 27af25b42c9..4f4cc279d91 100644 --- a/internal/gengapic/snippets_test.go +++ b/internal/gengapic/snippets_test.go @@ -30,18 +30,18 @@ import ( func TestSnippetsOutDir(t *testing.T) { for _, tst := range []struct { - opts options + cfg *generatorConfig want string }{ { - opts: options{ + cfg: &generatorConfig{ outDir: sample.GoPackagePath, pkgPath: sample.GoPackagePath, }, want: sample.SnippetsDirectory, }, { - opts: options{ + cfg: &generatorConfig{ outDir: "example.com/my/package", pkgPath: "example.com/my/package", }, @@ -49,9 +49,9 @@ func TestSnippetsOutDir(t *testing.T) { }, } { var g generator - g.opts = &tst.opts + g.cfg = tst.cfg if s := g.snippetsOutDir(); s != tst.want { - t.Errorf("TestGenAndCommitSnippets(g.opts.pkgPath = %s): got %s, want %s", g.opts.pkgPath, s, tst.want) + t.Errorf("TestGenAndCommitSnippets(g.opts.pkgPath = %s): got %s, want %s", g.cfg.pkgPath, s, tst.want) } } } @@ -109,7 +109,7 @@ func TestGenAndCommitSnippets(t *testing.T) { } var g generator - g.opts = &options{ + g.cfg = &generatorConfig{ pkgName: "pkg", transports: []transport{grpc}, } From a28ae8acc8c7f80665fc6eec9b99c1fa9d8ccdb9 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 15 Jan 2026 22:18:30 +0000 Subject: [PATCH 2/3] cleanups --- internal/gengapic/client_init_test.go | 15 +++++++-------- internal/gengapic/options.go | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index 39fc8828899..1c2993e9b6c 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -576,16 +576,15 @@ func TestClientInit(t *testing.T) { tst.serv.GetMethod()[0]: "Does some stuff.", } g.mixins = tst.mixins - g.cfg = &generatorConfig{ - APIServiceConfig: &serviceconfig.Service{ - Apis: []*apipb.Api{ - {Name: "foo.bar.Baz"}, - {Name: "google.iam.v1.IAMPolicy"}, - {Name: "google.cloud.location.Locations"}, - {Name: "google.longrunning.Operations"}, - }, + g.cfg.APIServiceConfig = &serviceconfig.Service{ + Apis: []*apipb.Api{ + {Name: "foo.bar.Baz"}, + {Name: "google.iam.v1.IAMPolicy"}, + {Name: "google.cloud.location.Locations"}, + {Name: "google.longrunning.Operations"}, }, } + g.aux.customOp = &customOp{ message: cop, } diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index d2a95267d89..6de78ce2bf9 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -308,7 +308,7 @@ func withAPIServiceConfigPath(s string) configOption { sc := &serviceconfig.Service{} if err := (protojson.UnmarshalOptions{DiscardUnknown: true}).Unmarshal(j, sc); err != nil { - fmt.Errorf("error unmarshaling api service config: %v", err) + return fmt.Errorf("error unmarshaling api service config: %v", err) } // An API Service Config will always have a `name` so if it is not populated, From 2b824f9c236e95d4fdac09f9838adddb533201a5 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 22 Jan 2026 23:14:13 +0000 Subject: [PATCH 3/3] address reviewer feedback --- internal/gengapic/generator.go | 3 ++- internal/gengapic/options.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index d1f0d9490d0..8e3689b672b 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -87,7 +87,8 @@ type generator struct { aux *auxTypes // Options for the generator determining module names, transports, - // config file paths, etc. + // config file paths, etc. This should be treated as immutable once + // configured. cfg *generatorConfig // GapicMetadata for recording proto-to-code mappings in a diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index 6de78ce2bf9..915f4709597 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -128,7 +128,7 @@ type generatorConfig struct { // Config options that return errors should not modify the configuration. type configOption func(*generatorConfig) error -// NewOptionsFromParams consumes the "parameter" field from the CodeGenerationRequest to produce a configuration. +// configFromRequest consumes the "parameter" field from the CodeGenerationRequest to produce a configuration. // This should be a comma seperated list of plugin arguments, and each one is handled in the order it appears. // // All plugin arguments understood by this plugin should be registered in one of the well known option collections, which