From ce9d9459e1d93d2f798237f11645629c6d92ce61 Mon Sep 17 00:00:00 2001 From: zwcn Date: Mon, 28 Jan 2019 15:46:10 -0500 Subject: [PATCH] protoc-gen-swagger: add the package name to the tags field of each endpoint if the package name exists in the proto file (#860) * add the package name to the tags field of each endpoint * prepend the package name only if it is defined in the proto file * fix the generate stage of the CI by checking in generated files * add a boolean flag allow_package_name_in_tags and unit tests * update bazel files * rename the flag to include_package_in_tags --- .../descriptor/registry.go | 16 +++++++ protoc-gen-swagger/genswagger/template.go | 8 +++- protoc-gen-swagger/main.go | 9 ++++ protoc-gen-swagger/main_test.go | 43 ++++++++++++------- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/protoc-gen-grpc-gateway/descriptor/registry.go b/protoc-gen-grpc-gateway/descriptor/registry.go index 9a201112b0a..510ab253830 100644 --- a/protoc-gen-grpc-gateway/descriptor/registry.go +++ b/protoc-gen-grpc-gateway/descriptor/registry.go @@ -50,6 +50,10 @@ type Registry struct { // 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 + // repeatedPathParamSeparator specifies how path parameter repeated fields are separated repeatedPathParamSeparator repeatedFieldSeparator @@ -350,6 +354,18 @@ 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) { + r.includePackageInTags = allow +} + +// IsIncludePackageInTags checks 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) IsIncludePackageInTags() bool { + return r.includePackageInTags +} + // GetRepeatedPathParamSeparator returns a rune spcifying how // path parameter repeated fields are separated. func (r *Registry) GetRepeatedPathParamSeparator() rune { diff --git a/protoc-gen-swagger/genswagger/template.go b/protoc-gen-swagger/genswagger/template.go index 771a005c99b..fcc98b1f51f 100644 --- a/protoc-gen-swagger/genswagger/template.go +++ b/protoc-gen-swagger/genswagger/template.go @@ -820,8 +820,14 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re // Use the streamdefinition which wraps the message in a "result" responseSchema.Ref = strings.Replace(responseSchema.Ref, `#/definitions/`, `#/x-stream-definitions/`, 1) } + + tag := svc.GetName() + if pkg := svc.File.GetPackage(); pkg != "" && reg.IsIncludePackageInTags() { + tag = pkg + "." + tag + } + operationObject := &swaggerOperationObject{ - Tags: []string{svc.GetName()}, + Tags: []string{tag}, Parameters: parameters, Responses: swaggerResponsesObject{ "200": swaggerResponseObject{ diff --git a/protoc-gen-swagger/main.go b/protoc-gen-swagger/main.go index a4d11fd1ef5..16ce528b709 100644 --- a/protoc-gen-swagger/main.go +++ b/protoc-gen-swagger/main.go @@ -25,6 +25,7 @@ var ( 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 verison") 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") + 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") ) // Variables set by goreleaser at build time @@ -74,6 +75,7 @@ func main() { reg.SetMergeFileName(*mergeFileName) reg.SetUseJSONNamesForFields(*useJSONNamesForFields) reg.SetAllowRepeatedFieldsInBody(*allowRepeatedFieldsInBody) + reg.SetIncludePackageInTags(*includePackageInTags) if err := reg.SetRepeatedPathParamSeparator(*repeatedPathParamSeparator); err != nil { emitError(err) return @@ -168,6 +170,13 @@ func parseReqParam(param string, f *flag.FlagSet, pkgMap map[string]string) erro } continue } + if spec[0] == "include_package_in_tags" { + err := f.Set(spec[0], "true") + if err != nil { + return fmt.Errorf("Cannot set flag %s: %v", p, err) + } + continue + } err := f.Set(spec[0], "") if err != nil { return fmt.Errorf("Cannot set flag %s: %v", p, err) diff --git a/protoc-gen-swagger/main_test.go b/protoc-gen-swagger/main_test.go index dc8e09bdb12..b5ebda041a2 100644 --- a/protoc-gen-swagger/main_test.go +++ b/protoc-gen-swagger/main_test.go @@ -17,6 +17,7 @@ func TestParseReqParam(t *testing.T) { allowDeleteBodyV bool allowMergeV bool allowRepeatedFieldsInBodyV bool + includePackageInTagsV bool fileV string importPathV string mergeFileNameV string @@ -28,35 +29,35 @@ func TestParseReqParam(t *testing.T) { name: "Test 0", expected: map[string]string{}, request: "", - allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, + allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, includePackageInTagsV: false, fileV: "-", importPathV: "", mergeFileNameV: "apidocs", }, { name: "Test 1", expected: map[string]string{"google/api/annotations.proto": "github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api"}, - request: "allow_delete_body,allow_merge,allow_repeated_fields_in_body,file=./foo.pb,import_prefix=/bar/baz,Mgoogle/api/annotations.proto=github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api", - allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: true, + request: "allow_delete_body,allow_merge,allow_repeated_fields_in_body,include_package_in_tags,file=./foo.pb,import_prefix=/bar/baz,Mgoogle/api/annotations.proto=github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api", + allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: true, includePackageInTagsV: true, fileV: "./foo.pb", importPathV: "/bar/baz", mergeFileNameV: "apidocs", }, { name: "Test 2", expected: map[string]string{"google/api/annotations.proto": "github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api"}, - request: "allow_delete_body=true,allow_merge=true,allow_repeated_fields_in_body=true,merge_file_name=test_name,file=./foo.pb,import_prefix=/bar/baz,Mgoogle/api/annotations.proto=github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api", - allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: true, + request: "allow_delete_body=true,allow_merge=true,allow_repeated_fields_in_body=true,include_package_in_tags=true,merge_file_name=test_name,file=./foo.pb,import_prefix=/bar/baz,Mgoogle/api/annotations.proto=github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis/google/api", + allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: true, includePackageInTagsV: true, fileV: "./foo.pb", importPathV: "/bar/baz", mergeFileNameV: "test_name", }, { name: "Test 3", expected: map[string]string{"a/b/c.proto": "github.com/x/y/z", "f/g/h.proto": "github.com/1/2/3/"}, request: "allow_delete_body=false,allow_merge=false,Ma/b/c.proto=github.com/x/y/z,Mf/g/h.proto=github.com/1/2/3/", - allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, + allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, includePackageInTagsV: false, fileV: "stdin", importPathV: "", mergeFileNameV: "apidocs", }, { name: "Test 4", expected: map[string]string{}, request: "", - allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, + allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, includePackageInTagsV: false, fileV: "stdin", importPathV: "", mergeFileNameV: "apidocs", }, { @@ -64,7 +65,7 @@ func TestParseReqParam(t *testing.T) { expected: map[string]string{}, request: "unknown_param=17", expectedError: errors.New("Cannot set flag unknown_param=17: no such flag -unknown_param"), - allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, + allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, includePackageInTagsV: false, fileV: "stdin", importPathV: "", mergeFileNameV: "apidocs", }, { @@ -72,14 +73,14 @@ func TestParseReqParam(t *testing.T) { expected: map[string]string{}, request: "Mfoo", expectedError: errors.New("Cannot set flag Mfoo: no such flag -Mfoo"), - allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, + allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, includePackageInTagsV: false, fileV: "stdin", importPathV: "", mergeFileNameV: "apidocs", }, { name: "Test 7", expected: map[string]string{}, - request: "allow_delete_body,file,import_prefix,allow_merge,allow_repeated_fields_in_body,merge_file_name", - allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: true, + request: "allow_delete_body,file,import_prefix,allow_merge,allow_repeated_fields_in_body,include_package_in_tags,merge_file_name", + allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: true, includePackageInTagsV: true, fileV: "", importPathV: "", mergeFileNameV: "", }, { @@ -87,9 +88,17 @@ func TestParseReqParam(t *testing.T) { expected: map[string]string{}, request: "allow_delete_body,file,import_prefix,allow_merge,allow_repeated_fields_in_body=3,merge_file_name", expectedError: errors.New(`Cannot set flag allow_repeated_fields_in_body=3: strconv.ParseBool: parsing "3": invalid syntax`), - allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: false, + allowDeleteBodyV: true, allowMergeV: true, allowRepeatedFieldsInBodyV: false, includePackageInTagsV: false, fileV: "", importPathV: "", mergeFileNameV: "apidocs", }, + { + name: "Test 9", + expected: map[string]string{}, + request: "include_package_in_tags=3", + expectedError: errors.New(`Cannot set flag include_package_in_tags=3: strconv.ParseBool: parsing "3": invalid syntax`), + allowDeleteBodyV: false, allowMergeV: false, allowRepeatedFieldsInBodyV: false, includePackageInTagsV: false, + fileV: "stdin", importPathV: "", mergeFileNameV: "apidocs", + }, } for i, tc := range testcases { @@ -112,10 +121,10 @@ func TestParseReqParam(t *testing.T) { tt.Errorf("pkgMap parse error, expected '%v', got '%v'", tc.expected, pkgMap) } if err.Error() != tc.expectedError.Error() { - tt.Errorf("expected error malformed, expected %q, go %q", tc.expectedError.Error(), err.Error()) + tt.Errorf("expected error malformed, expected %q, got %q", tc.expectedError.Error(), err.Error()) } } - checkFlags(tc.allowDeleteBodyV, tc.allowMergeV, tc.allowRepeatedFieldsInBodyV, tc.fileV, tc.importPathV, tc.mergeFileNameV, tt, i) + checkFlags(tc.allowDeleteBodyV, tc.allowMergeV, tc.allowRepeatedFieldsInBodyV, tc.includePackageInTagsV, tc.fileV, tc.importPathV, tc.mergeFileNameV, tt, i) clearFlags() }) @@ -123,7 +132,7 @@ func TestParseReqParam(t *testing.T) { } -func checkFlags(allowDeleteV, allowMergeV, allowRepeatedFieldsInBodyV bool, fileV, importPathV, mergeFileNameV string, t *testing.T, tid int) { +func checkFlags(allowDeleteV, allowMergeV, allowRepeatedFieldsInBodyV, includePackageInTagsV bool, fileV, importPathV, mergeFileNameV string, t *testing.T, tid int) { if *importPrefix != importPathV { t.Errorf("Test %v: import_prefix misparsed, expected '%v', got '%v'", tid, importPathV, *importPrefix) } @@ -142,6 +151,9 @@ func checkFlags(allowDeleteV, allowMergeV, allowRepeatedFieldsInBodyV bool, file if *allowRepeatedFieldsInBody != allowRepeatedFieldsInBodyV { t.Errorf("Test %v: allow_repeated_fields_in_body misparsed, expected '%v', got '%v'", tid, allowRepeatedFieldsInBodyV, *allowRepeatedFieldsInBody) } + if *includePackageInTags != includePackageInTagsV { + t.Errorf("Test %v: allow_repeated_fields_in_body misparsed, expected '%v', got '%v'", tid, includePackageInTagsV, *includePackageInTags) + } } func clearFlags() { @@ -150,5 +162,6 @@ func clearFlags() { *allowDeleteBody = false *allowMerge = false *allowRepeatedFieldsInBody = false + *includePackageInTags = false *mergeFileName = "apidocs" }