Repository: googleapis/gapic-generator-go
Pull request #1022 (2022-06-07 21:23:34)

feat(gengapic): rest-numeric-enums option enables enum-encoding sys param
For REGAPIC API calls, when the `rest-numeric-enums` plugin option is provided, include the new system parameter variant of `$alt=json` that enables proto-json encoding of enums in server responses as integers - `enum-encoding=int`.  The default is false. Includes the bazel attribute as well.

Also updates the README with option documentation.

Requested in http://b/232457394.

vchudnov-g: Oh, one question: are you planning on encoding all enums as ints when sending the request too? We think that would be desirable for symmetry.
noahdietz: > Oh, one question: are you planning on encoding all enums as ints when sending the request too? We think that would be desirable for symmetry.

That was already done in #994 :)
vchudnov-g: Oh, right! Great!
noahdietz: @vchudnov-g are we good to merge/release this yet? Thanks
vchudnov-g: @noahdietz No, let's not merge this yet. The server changes have not been rolled out yet.

Patch:
diff --git a/README.md b/README.md
index 3f18210..4b144dc 100644
--- a/README.md
+++ b/README.md
@@ -93,6 +93,14 @@ The configuration supported by the plugin option includes:
   * `api-service-config`: the path the service YAML file.
     * This is used for service-level client documentation.
 
+  * `transport`: the desired transport(s) to generate, delimited by `+` e.g. `grpc+rest`.
+    * Acceptable values are `grpc` and `rest`.
+    * Defaults to `grpc`.
+
+  * `rest-numeric-enums`: enables requesting response enums be encoded as numbers.
+    * Not enabled by default.
+    * Only effective when `rest` is included as a `transport` to be generated.
+
 Bazel
 -----
 
@@ -148,6 +156,15 @@ following attributes:
 
   * `metadata`: if `True`, [GapicMetadata](https://github.com/googleapis/googleapis/blob/master/gapic/metadata/gapic_metadata.proto) will be generated in JSON form. The default is `False`.
 
+  * `transport`: the desired transport(s) to generate, delimited by `+` e.g. `grpc+rest`.
+    * Acceptable values are `grpc` and `rest`.
+    * Defaults to `grpc`.
+
+  * `rest_numeric_enums`: if `True`, enables generation of system parameter requesting
+  response enums be encoded as numbers.
+    * Default is `False`.
+    * Only effective when `rest` is included as a `transport` to be generated.
+
 Docker Wrapper
 --------------
 The generator can also be executed via a Docker container. The image containes `protoc`, the microgenerator
diff --git a/internal/gengapic/genrest.go b/internal/gengapic/genrest.go
index ea8c7d2..8e2358e 100644
--- a/internal/gengapic/genrest.go
+++ b/internal/gengapic/genrest.go
@@ -380,9 +380,6 @@ func (g *generator) getLeafs(msg *descriptor.DescriptorProto, excludedFields ...
 func (g *generator) generateQueryString(m *descriptor.MethodDescriptorProto) {
 	p := g.printf
 	queryParams := g.queryParams(m)
-	if len(queryParams) == 0 {
-		return
-	}
 
 	// We want to iterate over fields in a deterministic order
 	// to prevent spurious deltas when regenerating gapics.
@@ -392,8 +389,13 @@ func (g *generator) generateQueryString(m *descriptor.MethodDescriptorProto) {
 	}
 	sort.Strings(fields)
 
-	g.imports[pbinfo.ImportSpec{Path: "net/url"}] = true
-	p("params := url.Values{}")
+	if g.opts.restNumericEnum || len(fields) > 0 {
+		g.imports[pbinfo.ImportSpec{Path: "net/url"}] = true
+		p("params := url.Values{}")
+	}
+	if g.opts.restNumericEnum {
+		p(`params.Add("$alt", "json;enum-encoding=int")`)
+	}
 	for _, path := range fields {
 		field := queryParams[path]
 		required := isRequired(field)
@@ -438,9 +440,12 @@ func (g *generator) generateQueryString(m *descriptor.MethodDescriptorProto) {
 		p("    %s", paramAdd)
 		p("}")
 	}
-	p("")
-	p("baseUrl.RawQuery = params.Encode()")
-	p("")
+
+	if g.opts.restNumericEnum || len(fields) > 0 {
+		p("")
+		p("baseUrl.RawQuery = params.Encode()")
+		p("")
+	}
 }
 
 func (g *generator) generateBaseURL(info *httpInfo, ret string) {
diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go
index 6c82eea..3a0691e 100644
--- a/internal/gengapic/options.go
+++ b/internal/gengapic/options.go
@@ -43,6 +43,7 @@ type options struct {
 	transports        []transport
 	metadata          bool
 	diregapic         bool
+	restNumericEnum   bool
 	pkgOverrides      map[string]string
 }
 
@@ -87,6 +88,9 @@ func parseOptions(parameter *string) (*options, error) {
 		case "diregapic":
 			opts.diregapic = true
 			continue
+		case "rest-numeric-enums":
+			opts.restNumericEnum = true
+			continue
 		}
 
 		e := strings.IndexByte(s, '=')
diff --git a/rules_go_gapic/go_gapic.bzl b/rules_go_gapic/go_gapic.bzl
index eeaa26b..1b4dfb3 100644
--- a/rules_go_gapic/go_gapic.bzl
+++ b/rules_go_gapic/go_gapic.bzl
@@ -93,6 +93,7 @@ def go_gapic_library(
   metadata = False,
   transport = "grpc",
   diregapic = False,
+  rest_numeric_enums = False,
   **kwargs):
 
   file_args = {}
@@ -117,6 +118,9 @@ def go_gapic_library(
   if diregapic:
     plugin_args.append("diregapic")
 
+  if rest_numeric_enums:
+    plugin_args.append("rest-numeric-enums")
+
   srcjar_name = name+"_srcjar"
   raw_srcjar_name = srcjar_name+"_raw"
   output_suffix = ".srcjar"
diff --git a/showcase/showcase.bash b/showcase/showcase.bash
index 64cd06e..7ac3c7e 100755
--- a/showcase/showcase.bash
+++ b/showcase/showcase.bash
@@ -48,6 +48,7 @@ protoc \
 	--go_out=plugins=grpc:./gen \
 	--go_gapic_out ./gen \
 	--go_gapic_opt 'transport=rest+grpc' \
+	--go_gapic_opt 'rest-numeric-enums' \
 	--go_gapic_opt 'go-gapic-package=github.com/googleapis/gapic-showcase/client;client' \
 	--go_gapic_opt 'grpc-service-config=showcase_grpc_service_config.json' \
 	--go_gapic_opt 'api-service-config=showcase_v1beta1.yaml' \


Test patch:
diff --git a/internal/gengapic/genrest_test.go b/internal/gengapic/genrest_test.go
index 8c4b556..ed3a367 100644
--- a/internal/gengapic/genrest_test.go
+++ b/internal/gengapic/genrest_test.go
@@ -698,7 +698,7 @@ func TestGenRestMethod(t *testing.T) {
 		{
 			name:    "unary_rpc",
 			method:  unaryRPC,
-			options: &options{},
+			options: &options{restNumericEnum: true},
 			imports: map[pbinfo.ImportSpec]bool{
 				{Path: "bytes"}: true,
 				{Path: "fmt"}:   true,
diff --git a/internal/gengapic/options_test.go b/internal/gengapic/options_test.go
index 8dc7d40..031e932 100644
--- a/internal/gengapic/options_test.go
+++ b/internal/gengapic/options_test.go
@@ -86,6 +86,17 @@ func TestParseOptions(t *testing.T) {
 			},
 			expectErr: false,
 		},
+		{
+			param: "transport=rest,rest-numeric-enums,go-gapic-package=path;pkg",
+			expectedOpts: &options{
+				transports:      []transport{rest},
+				pkgPath:         "path",
+				pkgName:         "pkg",
+				outDir:          "path",
+				restNumericEnum: true,
+			},
+			expectErr: false,
+		},
 		{
 			param:     "transport=tcp,go-gapic-package=path;pkg",
 			expectErr: true,
@@ -131,7 +142,7 @@ func TestParseOptions(t *testing.T) {
 		}
 
 		if !reflect.DeepEqual(opts, tst.expectedOpts) {
-			t.Errorf("parseOptions(%s) = %v, expected %v", tst.param, opts, tst.expectedOpts)
+			t.Errorf("parseOptions(%s) = %+v, expected %+v", tst.param, opts, tst.expectedOpts)
 			continue
 		}
 	}
diff --git a/internal/gengapic/testdata/rest_UnaryRPC.want b/internal/gengapic/testdata/rest_UnaryRPC.want
index be7f7f3..95aad1a 100644
--- a/internal/gengapic/testdata/rest_UnaryRPC.want
+++ b/internal/gengapic/testdata/rest_UnaryRPC.want
@@ -11,6 +11,11 @@ func (c *fooRESTClient) UnaryRPC(ctx context.Context, req *foopb.Foo, opts ...ga
 	}
 	baseUrl.Path += fmt.Sprintf("/v1/foo")
 
+	params := url.Values{}
+	params.Add("$alt", "json;enum-encoding=int")
+
+	baseUrl.RawQuery = params.Encode()
+
 	// Build HTTP headers from client and context metadata.
 	routingHeaders := ""
 	routingHeadersMap := make(map[string]string)