diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index aa7588e37d5..35a67f06f95 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -121,7 +121,7 @@ func newGenerator(req *pluginpb.CodeGeneratorRequest) (*generator, error) { }, } - opts, err := parseOptions(req.Parameter) + opts, err := newOptionsFromParams(req.Parameter) if err != nil { return nil, err } diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index b353431697c..d73032876fd 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -22,163 +22,324 @@ import ( "strings" ) +// Define a type to represent supported network transports. type transport int +// Declare the transports we support for generation. const ( grpc transport = iota rest ) -var errInvalidParam = errors.New("need parameter in format: go-gapic-package=client/import/path;packageName") +// static error for the most critical argument +var errInvalidPackageParam = errors.New("need parameter in format: go-gapic-package=client/import/path;packageName") -type options struct { - pkgPath string - pkgName string - outDir string - relLvl string - modulePrefix string - grpcConfPath string - serviceConfigPath string - transports []transport - metadata bool - diregapic bool - restNumericEnum bool - pkgOverrides map[string]string - omitSnippets bool -} - -// 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) -// * api-service-config (filepath) -// * grpc-service-config (filepath) -// * module (name) -// * Mfile=import (e.g. Mgoogle/storage/v2/storage.proto=cloud.google.com/go/storage/internal/apiv2/stubs) -// * release-level (one of 'alpha', 'beta', 'deprecated', or empty) -// * transport ('+' separated list of transport backends to generate) -// * metadata (enable GAPIC metadata generation) -// * omit-snippets (skip example code snippets generation to the `internal/generated/snippets` path) -// The only required option is 'go-gapic-package'. +// DeprecatedArgs are plugin arguments we want to explicitly error on, so invokers know that the +// option is no longer supported. +var DeprecatedArgs map[string]error = map[string]error{ + "gapic-service-config": errors.New("removed, use api-service-config instead"), +} + +// SupportedBooleanArgs expose boolean plugin arguments (presence enables option). +var SupportedBooleanArgs map[string]func() configOption = map[string]func() configOption{ + "metadata": enableGAPICMetadata, + "diregapic": generateAsDIREGAPIC, + "rest-numeric-enums": enableRESTNumericEnums, + "omit-snippets": enableOmitSnippets, +} + +// SupportedValueArgs are arguments that are supplied in the form =. +// The map is keyed by the argument key. +var SupportedValueArgs map[string]func(string) configOption = map[string]func(string) configOption{ + "go-gapic-package": withGoGAPICPackage, + "api-service-config": withAPIServiceConfigPath, + "grpc-service-config": withGRPCServiceConfigPath, + "module": withModulePrefix, + "release-level": withReleaseLevel, + "transport": withTransports, +} + +// SupportedPrefixArgs are a special case of the value arg that use a string prefix. +// The primary use is for dynamic package overrides, which can be used to override the package +// for a particular input in the example form: // -// Valid parameter example: -// go-gapic-package=path/to/out;pkg,module=path,transport=rest+grpc,api-service-config=api_v1.yaml,release-level=alpha +// Mgoogle/storage/v2/storage.proto=cloud.google.com/go/storage/internal/apiv2/stubs // -// 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{} +// Recommendation: avoid adding more of these as they complicate processing. +var SupportedPrefixArgs map[string]func(string) configOption = map[string]func(string) configOption{ + "M": withPackageOverride, +} + +// 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 { + // The fully qualified package path, e.g. "cloud.google.com/go/foo/v1/foopb" + pkgPath string + // The package name, e.g. "foopb" + pkgName string + // Output directory for the generated artifacts. + outDir string + + // Should GAPIC metadata be generated + // TODO: rename this in a subsequent refactor + metadata bool + + // Are the input artifacts from a DIREGAPIC source + // TODO: rename this in a subsequent refactor + diregapic bool + + // Should the generator code generator numeric enums for REST calls + // TODO: rename this in a subsquent refactor + restNumericEnum bool + + // should generated snippets be omitted (generated by default) + // TODO: rename this in a subsequent refactor + omitSnippets bool + + // path to the API service config + // TODO: rename and handle parsing during configuration + serviceConfigPath string + + // path to the GRPC service config (e.g. method retry settings) + // TODO: rename and handle parsing during configuration + grpcConfPath string + + // prefix for the enclosing module + modulePrefix string + + // release level of the generated artifacts + // TODO: rename this in a subsequent refactor + relLvl string + + // which network transports are being generated. + // TODO: rename this and make it easier to get right (likely a map) in a refactor + transports []transport + + // Package naming overrides, keyed by filepath. + pkgOverrides map[string]string +} + +// 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 - if parameter == nil { - return nil, fmt.Errorf("empty options parameter") +// 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. +// +// All plugin arguments understood by this plugin should be registered in one of the well known option collections, which +// are used to process the input parameters: +// DeprecatedArgs +// SupportedBooleanArgs +// SupportedValueArgs +// SupportedPrefixArgs +func newOptionsFromParams(generationParameter *string) (*options, error) { + if generationParameter == nil { + return nil, errors.New("generationParameter is nil, cannot configure") } - // parse plugin params, ignoring unknown values - for _, s := range strings.Split(*parameter, ",") { - // skip empty --go_gapic_opt flags - if s == "" { - continue - } + cfg := &options{} - // Check for boolean flags. - switch s { - case "metadata": - opts.metadata = true - continue - case "diregapic": - opts.diregapic = true - continue - case "rest-numeric-enums": - opts.restNumericEnum = true - continue - case "omit-snippets": - opts.omitSnippets = true - continue - } + // params are comma seperated. Split and process each individually. + for _, s := range strings.Split(*generationParameter, ",") { + // Normalize to ensure we're not dealing with spacing issues. + pluginArg := strings.TrimSpace(s) - e := strings.IndexByte(s, '=') - if e < 0 { - return nil, fmt.Errorf("invalid plugin option format, must be key=value: %q", s) + // Ignore empty args. + if pluginArg == "" { + continue } - key, val := s[:e], s[e+1:] - if val == "" { - return nil, fmt.Errorf("invalid plugin option value, missing value in key=value: %q", s) + // Handle deprecated arguments first. + if err, ok := DeprecatedArgs[pluginArg]; ok { + return nil, err } - - switch key { - case "go-gapic-package": - p := strings.IndexByte(s, ';') - - if p < 0 { - return nil, errInvalidParam + // Handle the boolean keyword args, e.g. non key=value style arguments. + if o, ok := SupportedBooleanArgs[pluginArg]; ok { + if err := o()(cfg); err != nil { + // It's unlikely that any boolean option will surface errors, but we do it here + // lest we are surprised in the future. + return nil, fmt.Errorf("plugin arg %q invalid: %w", pluginArg, err) } + continue + } - opts.pkgPath = s[e+1 : p] - opts.pkgName = s[p+1:] - opts.outDir = filepath.FromSlash(opts.pkgPath) - case "gapic-service-config": - // Deprecated: this option is deprecated and will be removed in a - // later release. - fallthrough - case "api-service-config": - opts.serviceConfigPath = val - case "grpc-service-config": - opts.grpcConfPath = val - case "module": - opts.modulePrefix = val - case "release-level": - opts.relLvl = strings.ToLower(val) - case "transport": - // Prevent duplicates - transports := map[transport]bool{} - for _, t := range strings.Split(val, "+") { - switch t { - case "grpc": - transports[grpc] = true - case "rest": - transports[rest] = true - default: - return nil, fmt.Errorf("invalid transport option: %q", t) + // Handle value args by parsing key=value parts. + e := strings.IndexByte(pluginArg, '=') + if e > 0 { + key, val := pluginArg[:e], pluginArg[e+1:] + if valueOpt, ok := SupportedValueArgs[key]; ok { + if err := valueOpt(val)(cfg); err != nil { + return nil, fmt.Errorf("plugin arg %q invalid: %w", pluginArg, err) } + continue } - for t := range transports { - opts.transports = append(opts.transports, t) - } - sort.Slice(opts.transports, func(i, j int) bool { - return opts.transports[i] < opts.transports[j] - }) - default: - // go_package override for the protobuf/grpc stubs. - // Mgoogle/storage/v2/storage.proto=cloud.google.com/go/storage/internal/apiv2/stubs - if key[0] == 'M' { - file := key[1:] - if opts.pkgOverrides == nil { - opts.pkgOverrides = make(map[string]string) + } + + // Now, handle prefix args by scanning registered prefixes. + for prefix, prefixOpt := range SupportedPrefixArgs { + if strings.HasPrefix(pluginArg, prefix) { + prefixLen := len(prefix) + if err := prefixOpt(pluginArg[prefixLen:])(cfg); err != nil { + return nil, fmt.Errorf("plugin arg %q invalid: %w", pluginArg, err) } - opts.pkgOverrides[file] = val + continue } } + + } + // Now that all options are processed, normalized and validate. + if err := validateAndNormalizeOptions(cfg); err != nil { + return nil, err + } + return cfg, nil +} + +// This function provides an opportunity to validate that a configuration adheres to expectations +// and the configuration adheres to expectations (e.g. required fields are present, fields with relationships +// are not in conflict). +// +// 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 { + // Normalize transports. If no transports are specified, generate gRPC only by default. + if len(cfg.transports) == 0 { + cfg.transports = []transport{grpc} } - if opts.pkgPath == "" || opts.pkgName == "" || opts.outDir == "" { - return nil, errInvalidParam + // REST enums are not supported by DIREGAPIC. + if cfg.diregapic && cfg.restNumericEnum { + return errors.New("incompatible features: diregapic and rest numeric enums") } - if opts.modulePrefix != "" { - if !strings.HasPrefix(opts.outDir, opts.modulePrefix) { - return nil, fmt.Errorf("go-gapic-package %q does not match prefix %q", opts.outDir, opts.modulePrefix) + // Certain configuration details must be present. + if cfg.pkgPath == "" || cfg.pkgName == "" || cfg.outDir == "" { + return errInvalidPackageParam + } + + if cfg.modulePrefix != "" { + if !strings.HasPrefix(cfg.outDir, cfg.modulePrefix) { + return fmt.Errorf("go-gapic-package %q does not match prefix %q", cfg.outDir, cfg.modulePrefix) + } + cfg.outDir = strings.TrimPrefix(cfg.outDir, cfg.modulePrefix+"/") + } + + return nil +} + +// withGoGapicPackage parses the value argument based on the typical +// pattern: +// ; +// Example: +// cloud.google.com/go/foo/v1/foopb;foopb +func withGoGAPICPackage(s string) configOption { + p := strings.IndexByte(s, ';') + + if p < 0 { + return func(cfg *options) error { + return errInvalidPackageParam } - opts.outDir = strings.TrimPrefix(opts.outDir, opts.modulePrefix+"/") } + return func(cfg *options) error { + cfg.pkgPath = s[0:p] + cfg.pkgName = s[p+1:] + cfg.outDir = filepath.FromSlash(cfg.pkgPath) + return nil + } +} + +// enableGAPICMetadata enables generation of GAPIC metadata. +func enableGAPICMetadata() configOption { + return func(cfg *options) error { + cfg.metadata = true + return nil + } +} + +// generateAsDIREGAPIC is a behavioral flag that ensures code generation respects +// special DIREGAPIC constraints. A DIREGAPIC is a set of input artifacts that were +// reverse-compiled from an API Discovery document, rather than directly from service +// protos (e.g. compute). +func generateAsDIREGAPIC() configOption { + return func(cfg *options) error { + cfg.diregapic = true + return nil + } +} + +// 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 { + cfg.restNumericEnum = true + return nil + } +} + +// Should automatic generation of snippets be disabled. +func enableOmitSnippets() configOption { + return func(cfg *options) error { + cfg.omitSnippets = true + return nil + } +} - // Default is just grpc for now. - if opts.transports == nil { - opts.transports = []transport{grpc} +// Specifies the path to the API service config file. +func withAPIServiceConfigPath(s string) configOption { + return func(cfg *options) error { + cfg.serviceConfigPath = s + return nil } +} - return &opts, nil +// Specifies the path to the gRPC service config file. +func withGRPCServiceConfigPath(s string) configOption { + return func(cfg *options) error { + cfg.grpcConfPath = s + return nil + } +} + +// Specifies the module prefix. +func withModulePrefix(s string) configOption { + return func(cfg *options) error { + cfg.modulePrefix = s + return nil + } +} + +// Specifies the release level of the generated artifacts. +func withReleaseLevel(s string) configOption { + return func(cfg *options) error { + // TODO: this should be validated against a well defined set of levels + cfg.relLvl = s + return nil + } +} + +// withPackageOverride handles a package naming override. +// The `M` prefix is not passed as part of the string, only the remaining substring. +func withPackageOverride(s string) configOption { + e := strings.IndexByte(s, '=') + if e < 0 { + return func(cfg *options) 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 fmt.Errorf("invalid plugin option value, missing value in key=value: %q", s) + } + } + return func(cfg *options) error { + if cfg.pkgOverrides == nil { + cfg.pkgOverrides = make(map[string]string) + } + cfg.pkgOverrides[key] = val + return nil + } } // Utility function for stringifying the Transport enum @@ -193,3 +354,31 @@ func (t transport) String() string { return fmt.Sprintf("%d", int(t)) } } + +// Specifies the requested network transports to generate. +func withTransports(s string) configOption { + transports := map[transport]bool{} + for _, t := range strings.Split(s, "+") { + switch t { + case "grpc": + transports[grpc] = true + case "rest": + transports[rest] = true + default: + return func(cfg *options) error { + return fmt.Errorf("invalid transport option: %q", t) + } + } + + } + return func(cfg *options) error { + for t := range transports { + cfg.transports = append(cfg.transports, t) + } + // TODO: remove this after refactor, its a weak expectation + sort.Slice(cfg.transports, func(i, j int) bool { + return cfg.transports[i] < cfg.transports[j] + }) + return nil + } +} diff --git a/internal/gengapic/options_test.go b/internal/gengapic/options_test.go index 1d21bc6cf00..637ce7ff57e 100644 --- a/internal/gengapic/options_test.go +++ b/internal/gengapic/options_test.go @@ -17,6 +17,8 @@ package gengapic import ( "reflect" "testing" + + "github.com/google/go-cmp/cmp" ) func TestParseOptions(t *testing.T) { @@ -140,7 +142,7 @@ func TestParseOptions(t *testing.T) { expectErr: false, }, } { - opts, err := parseOptions(&tst.param) + opts, err := newOptionsFromParams(&tst.param) if tst.expectErr && err == nil { t.Errorf("parseOptions(%s) expected error", tst.param) continue @@ -157,3 +159,73 @@ func TestParseOptions(t *testing.T) { } } } + +func TestValidateAndNormalizeOptions(t *testing.T) { + for _, tc := range []struct { + description string + in *options + wantErr bool + want *options + }{ + { + description: "empty", + in: &options{}, + wantErr: true, // no package info + }, + { + description: "minimal", + in: &options{ + pkgPath: "path", + pkgName: "name", + outDir: "dir", + }, + want: &options{ + pkgPath: "path", + pkgName: "name", + outDir: "dir", + transports: []transport{grpc}, + }, + }, + { + description: "conflict diregapic enum", + in: &options{ + pkgPath: "path", + pkgName: "name", + outDir: "dir", + diregapic: true, + restNumericEnum: true, + }, + wantErr: true, + }, + { + description: "mismatched module prefix", + in: &options{ + pkgPath: "path", + pkgName: "name", + outDir: "foo/bar/baz", + modulePrefix: "notfoo", + }, + wantErr: true, + }, + } { + t.Run(tc.description, func(t *testing.T) { + got := tc.in + err := validateAndNormalizeOptions(got) + if tc.wantErr { + if err != nil { + return + } + t.Fatalf("wanted err, got success") + } + if !tc.wantErr { + if err != nil { + t.Errorf("got unwanted err: %v", err) + } + } + if diff := cmp.Diff(got, tc.want, cmp.AllowUnexported(options{})); diff != "" { + t.Errorf("got(-), want(+):\n%s", diff) + } + }) + + } +}