Default allow_repeated_fields_in_body option to true and deprecate op…

…tion (#2813)

* Deprecate allow_repeated_fields_in_body option with default true

The allow_repeated_fields_in_body option is now ignored and always behaves as if it is set to true. Users who try to set the flag will see a warning.

* Delete unused parseFlags function and tests

internal/descriptor/registry.go

@@ -48,9 +48,6 @@ type Registry struct {
 	// mergeFileName target OpenAPI file name after merge
 	mergeFileName string
 
-	// allowRepeatedFieldsInBody permits repeated field in body field path of `google.api.http` annotation option
-	allowRepeatedFieldsInBody bool
-
 	// includePackageInTags controls whether the package name defined in the `package` directive
 	// in the proto file can be prepended to the gRPC service name in the `Tags` field of every operation.
 	includePackageInTags bool
@@ -451,18 +448,6 @@ func (r *Registry) SetMergeFileName(mergeFileName string) {
 	r.mergeFileName = mergeFileName
 }
 
-// SetAllowRepeatedFieldsInBody controls whether repeated field can be used
-// in `body` and `response_body` (`google.api.http` annotation option) field path or not
-func (r *Registry) SetAllowRepeatedFieldsInBody(allow bool) {
-	r.allowRepeatedFieldsInBody = allow
-}
-
-// IsAllowRepeatedFieldsInBody checks if repeated field can be used
-// in `body` and `response_body` (`google.api.http` annotation option) field path or not
-func (r *Registry) IsAllowRepeatedFieldsInBody() bool {
-	return r.allowRepeatedFieldsInBody
-}
-
 // SetIncludePackageInTags controls whether the package name defined in the `package` directive
 // in the proto file can be prepended to the gRPC service name in the `Tags` field of every operation.
 func (r *Registry) SetIncludePackageInTags(allow bool) {

internal/descriptor/services.go

@@ -325,9 +325,6 @@ func (r *Registry) resolveFieldPath(msg *Message, path string, isPathParam bool)
 		if f == nil {
 			return nil, fmt.Errorf("no field %q found in %s", path, root.GetName())
 		}
-		if !(isPathParam || r.allowRepeatedFieldsInBody) && f.GetLabel() == descriptorpb.FieldDescriptorProto_LABEL_REPEATED {
-			return nil, fmt.Errorf("repeated field not allowed in field path: %s in %s", f.GetName(), path)
-		}
 		if isPathParam && f.GetProto3Optional() {
 			return nil, fmt.Errorf("optional field not allowed in field path: %s in %s", f.GetName(), path)
 		}

internal/descriptor/services_test.go

@@ -1117,7 +1117,7 @@ func TestResolveFieldPath(t *testing.T) {
 				>
 			`,
 			path:    "string",
-			wantErr: true,
+			wantErr: false,
 		},
 		// nested field
 		{
@@ -1222,7 +1222,7 @@ func TestResolveFieldPath(t *testing.T) {
 				>
 			`,
 			path:    "nested.nested2.terminal",
-			wantErr: true,
+			wantErr: false,
 		},
 	} {
 		var file descriptorpb.FileDescriptorProto

protoc-gen-grpc-gateway/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
 load("@io_bazel_rules_go//proto:compiler.bzl", "go_proto_compiler")
 
 package(default_visibility = ["//visibility:private"])
@@ -42,10 +42,3 @@ go_proto_compiler(
         "@org_golang_google_protobuf//proto:go_default_library",
     ],
 )
-
-go_test(
-    name = "protoc-gen-grpc-gateway_test",
-    srcs = ["main_test.go"],
-    embed = [":protoc-gen-grpc-gateway_lib"],
-    deps = ["//internal/descriptor"],
-)

protoc-gen-grpc-gateway/main.go

@@ -26,7 +26,7 @@ var (
 	useRequestContext          = flag.Bool("request_context", true, "determine whether to use http.Request's context or not")
 	allowDeleteBody            = flag.Bool("allow_delete_body", false, "unless set, HTTP DELETE methods may not have a body")
 	grpcAPIConfiguration       = flag.String("grpc_api_configuration", "", "path to gRPC API Configuration in YAML format")
-	allowRepeatedFieldsInBody  = flag.Bool("allow_repeated_fields_in_body", false, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option")
+	_                          = flag.Bool("allow_repeated_fields_in_body", true, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option. DEPRECATED: the value is ignored and always behaves as `true`.")
 	repeatedPathParamSeparator = flag.String("repeated_path_param_separator", "csv", "configures how repeated fields should be split. Allowed values are `csv`, `pipes`, `ssv` and `tsv`.")
 	allowPatchFeature          = flag.Bool("allow_patch_feature", true, "determines whether to use PATCH feature involving update masks (using google.protobuf.FieldMask).")
 	omitPackageDoc             = flag.Bool("omit_package_doc", false, "if true, no package comment will be included in the generated code")
@@ -101,32 +101,6 @@ func main() {
 	})
 }
 
-func parseFlags(reg *descriptor.Registry, parameter string) {
-	if parameter == "" {
-		return
-	}
-
-	for _, p := range strings.Split(parameter, ",") {
-		spec := strings.SplitN(p, "=", 2)
-		if len(spec) == 1 {
-			if err := flag.CommandLine.Set(spec[0], ""); err != nil {
-				glog.Fatalf("Cannot set flag %s", p)
-			}
-			continue
-		}
-
-		name, value := spec[0], spec[1]
-
-		if strings.HasPrefix(name, "M") {
-			reg.AddPkgMap(name[1:], value)
-			continue
-		}
-		if err := flag.CommandLine.Set(name, value); err != nil {
-			glog.Fatalf("Cannot set flag %s", p)
-		}
-	}
-}
-
 func applyFlags(reg *descriptor.Registry) error {
 	if *grpcAPIConfiguration != "" {
 		if err := reg.LoadGrpcAPIServiceFromYAML(*grpcAPIConfiguration); err != nil {
@@ -138,7 +112,13 @@ func applyFlags(reg *descriptor.Registry) error {
 	}
 	reg.SetStandalone(*standalone)
 	reg.SetAllowDeleteBody(*allowDeleteBody)
-	reg.SetAllowRepeatedFieldsInBody(*allowRepeatedFieldsInBody)
+
+	flag.Visit(func(f *flag.Flag) {
+		if f.Name == "allow_repeated_fields_in_body" {
+			glog.Warning("The `allow_repeated_fields_in_body` flag is deprecated and will always behave as `true`.")
+		}
+	})
+
 	reg.SetOmitPackageDoc(*omitPackageDoc)
 	reg.SetWarnOnUnboundMethods(*warnOnUnboundMethods)
 	reg.SetGenerateUnboundMethods(*generateUnboundMethods)

protoc-gen-openapiv2/main.go

@@ -25,7 +25,7 @@ var (
 	useJSONNamesForFields          = flag.Bool("json_names_for_fields", true, "if disabled, the original proto name will be used for generating OpenAPI definitions")
 	repeatedPathParamSeparator     = flag.String("repeated_path_param_separator", "csv", "configures how repeated fields should be split. Allowed values are `csv`, `pipes`, `ssv` and `tsv`")
 	versionFlag                    = flag.Bool("version", false, "print the current version")
-	allowRepeatedFieldsInBody      = flag.Bool("allow_repeated_fields_in_body", false, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option")
+	_                              = flag.Bool("allow_repeated_fields_in_body", true, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option. DEPRECATED: the value is ignored and always behaves as `true`.")
 	includePackageInTags           = flag.Bool("include_package_in_tags", false, "if unset, the gRPC service name is added to the `Tags` field of each operation. If set and the `package` directive is shown in the proto file, the package name will be prepended to the service name")
 	useFQNForOpenAPIName           = flag.Bool("fqn_for_openapi_name", false, "if set, the object's OpenAPI names will use the fully qualified names from the proto definition (ie my.package.MyMessage.MyInnerMessage). DEPRECATED: prefer `openapi_naming_strategy=fqn`")
 	openAPINamingStrategy          = flag.String("openapi_naming_strategy", "", "use the given OpenAPI naming strategy. Allowed values are `legacy`, `fqn`, `simple`. If unset, either `legacy` or `fqn` are selected, depending on the value of the `fqn_for_openapi_name` flag")
@@ -88,7 +88,13 @@ func main() {
 	reg.SetAllowMerge(*allowMerge)
 	reg.SetMergeFileName(*mergeFileName)
 	reg.SetUseJSONNamesForFields(*useJSONNamesForFields)
-	reg.SetAllowRepeatedFieldsInBody(*allowRepeatedFieldsInBody)
+
+	flag.Visit(func(f *flag.Flag) {
+		if f.Name == "allow_repeated_fields_in_body" {
+			glog.Warning("The `allow_repeated_fields_in_body` flag is deprecated and will always behave as `true`.")
+		}
+	})
+
 	reg.SetIncludePackageInTags(*includePackageInTags)
 
 	reg.SetUseFQNForOpenAPIName(*useFQNForOpenAPIName)