Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default allow_repeated_fields_in_body option to true and deprecate option #2813

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions internal/descriptor/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 0 additions & 3 deletions internal/descriptor/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
johanbrandhorst marked this conversation as resolved.
Show resolved Hide resolved
if isPathParam && f.GetProto3Optional() {
return nil, fmt.Errorf("optional field not allowed in field path: %s in %s", f.GetName(), path)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/descriptor/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1117,7 +1117,7 @@ func TestResolveFieldPath(t *testing.T) {
>
`,
path: "string",
wantErr: true,
wantErr: false,
},
// nested field
{
Expand Down Expand Up @@ -1222,7 +1222,7 @@ func TestResolveFieldPath(t *testing.T) {
>
`,
path: "nested.nested2.terminal",
wantErr: true,
wantErr: false,
},
} {
var file descriptorpb.FileDescriptorProto
Expand Down
9 changes: 1 addition & 8 deletions protoc-gen-grpc-gateway/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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"])
Expand Down Expand Up @@ -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"],
)
36 changes: 8 additions & 28 deletions protoc-gen-grpc-gateway/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
31 changes: 0 additions & 31 deletions protoc-gen-grpc-gateway/main_test.go

This file was deleted.

10 changes: 8 additions & 2 deletions protoc-gen-openapiv2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
Loading