From 7129b7232445b9e36ad55fb4897ca2025bb601e5 Mon Sep 17 00:00:00 2001 From: y-takuya Date: Mon, 22 Aug 2022 01:54:18 +0900 Subject: [PATCH 1/3] fix: support service tags in OpenAPI config file --- .../clients/unannotatedecho/api/swagger.yaml | 5 +++ .../unannotated_echo_service.swagger.json | 7 ++- .../internal/genopenapi/template.go | 45 ++++++++++++++++--- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/examples/internal/clients/unannotatedecho/api/swagger.yaml b/examples/internal/clients/unannotatedecho/api/swagger.yaml index afa15b8757e..7d2f2d17244 100644 --- a/examples/internal/clients/unannotatedecho/api/swagger.yaml +++ b/examples/internal/clients/unannotatedecho/api/swagger.yaml @@ -17,6 +17,11 @@ info: x-something-something: "yadda" tags: - name: "UnannotatedEchoService" + description: "UnannotatedEchoService description -- which should not be used in\ + \ place of the documentation comment!" + externalDocs: + description: "Find out more about UnannotatedEchoService" + url: "https://github.com/grpc-ecosystem/grpc-gateway" schemes: - "http" - "https" diff --git a/examples/internal/proto/examplepb/unannotated_echo_service.swagger.json b/examples/internal/proto/examplepb/unannotated_echo_service.swagger.json index fc4f3011e36..f6c9c2c4f50 100644 --- a/examples/internal/proto/examplepb/unannotated_echo_service.swagger.json +++ b/examples/internal/proto/examplepb/unannotated_echo_service.swagger.json @@ -17,7 +17,12 @@ }, "tags": [ { - "name": "UnannotatedEchoService" + "name": "UnannotatedEchoService", + "description": "UnannotatedEchoService description -- which should not be used in place of the documentation comment!", + "externalDocs": { + "description": "Find out more about UnannotatedEchoService", + "url": "https://github.com/grpc-ecosystem/grpc-gateway" + } } ], "schemes": [ diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index fc4cf85fab7..fd08cd704e6 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -1007,14 +1007,13 @@ func renderServiceTags(services []*descriptor.Service, reg *descriptor.Registry) tag := openapiTagObject{ Name: tagName, } - if proto.HasExtension(svc.Options, openapi_options.E_Openapiv2Tag) { - ext := proto.GetExtension(svc.Options, openapi_options.E_Openapiv2Tag) - opts, ok := ext.(*openapi_options.Tag) - if !ok { - glog.Errorf("extension is %T; want an OpenAPI Tag object", ext) - return nil - } + opts, err := getServiceOpenAPIOption(reg, svc) + if err != nil { + glog.Error(err) + return nil + } + if opts != nil { tag.Description = opts.Description if opts.ExternalDocs != nil { tag.ExternalDocs = &openapiExternalDocumentationObject{ @@ -2351,6 +2350,23 @@ func extractSchemaOptionFromMessageDescriptor(msg *descriptorpb.DescriptorProto) return opts, nil } +// extractTagOptionFromServiceDescriptor extracts the tag of type +// openapi_options.Tag from a given proto service's descriptor. +func extractTagOptionFromServiceDescriptor(svc *descriptorpb.ServiceDescriptorProto) (*openapi_options.Tag, error) { + if svc.Options == nil { + return nil, nil + } + if !proto.HasExtension(svc.Options, openapi_options.E_Openapiv2Tag) { + return nil, nil + } + ext := proto.GetExtension(svc.Options, openapi_options.E_Openapiv2Tag) + opts, ok := ext.(*openapi_options.Tag) + if !ok { + return nil, fmt.Errorf("extension is %T; want a Tag", ext) + } + return opts, nil +} + // extractOpenAPIOptionFromFileDescriptor extracts the message of type // openapi_options.OpenAPI from a given proto method's descriptor. func extractOpenAPIOptionFromFileDescriptor(file *descriptorpb.FileDescriptorProto) (*openapi_options.Swagger, error) { @@ -2488,6 +2504,21 @@ func getMessageOpenAPIOption(reg *descriptor.Registry, msg *descriptor.Message) return opts, nil } +func getServiceOpenAPIOption(reg *descriptor.Registry, svc *descriptor.Service) (*openapi_options.Tag, error) { + opts, err := extractTagOptionFromServiceDescriptor(svc.ServiceDescriptorProto) + if err != nil { + return nil, err + } + if opts != nil { + return opts, nil + } + opts, ok := reg.GetOpenAPIServiceOption(svc.FQSN()) + if !ok { + return nil, nil + } + return opts, nil +} + func getFileOpenAPIOption(reg *descriptor.Registry, file *descriptor.File) (*openapi_options.Swagger, error) { opts, err := extractOpenAPIOptionFromFileDescriptor(file.FileDescriptorProto) if err != nil { From 5215131dfde4888b75f2c42526bc1a1d9a398308 Mon Sep 17 00:00:00 2001 From: y-takuya Date: Wed, 7 Sep 2022 00:38:43 +0900 Subject: [PATCH 2/3] chore: add test for service tags support --- .../internal/genopenapi/template_test.go | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index 94d75e39f7a..15e9aef8b78 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -1467,6 +1467,110 @@ func TestApplyTemplateMultiService(t *testing.T) { } } +func TestApplyTemplateOpenAPIConfigFromYAML(t *testing.T) { + msgdesc := &descriptorpb.DescriptorProto{ + Name: proto.String("ExampleMessage"), + } + meth := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("Example"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("ExampleMessage"), + } + svc := &descriptorpb.ServiceDescriptorProto{ + Name: proto.String("ExampleService"), + Method: []*descriptorpb.MethodDescriptorProto{meth}, + } + msg := &descriptor.Message{ + DescriptorProto: msgdesc, + } + file := descriptor.File{ + FileDescriptorProto: &descriptorpb.FileDescriptorProto{ + SourceCodeInfo: &descriptorpb.SourceCodeInfo{}, + Name: proto.String("example.proto"), + Package: proto.String("example"), + MessageType: []*descriptorpb.DescriptorProto{msgdesc}, + Service: []*descriptorpb.ServiceDescriptorProto{svc}, + Options: &descriptorpb.FileOptions{ + GoPackage: proto.String("github.com/grpc-ecosystem/grpc-gateway/runtime/internal/examplepb;example"), + }, + }, + GoPkg: descriptor.GoPackage{ + Path: "example.com/path/to/example/example.pb", + Name: "example_pb", + }, + Messages: []*descriptor.Message{msg}, + Services: []*descriptor.Service{ + { + ServiceDescriptorProto: svc, + Methods: []*descriptor.Method{ + { + MethodDescriptorProto: meth, + RequestType: msg, + ResponseType: msg, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + Body: &descriptor.Body{FieldPath: nil}, + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/echo", // TODO(achew22): Figure out what this should really be + }, + }, + }, + }, + }, + }, + }, + } + reg := descriptor.NewRegistry() + if err := AddErrorDefs(reg); err != nil { + t.Errorf("AddErrorDefs(%#v) failed with %v; want success", reg, err) + return + } + fileCL := crossLinkFixture(&file) + err := reg.Load(reqFromFile(fileCL)) + if err != nil { + t.Errorf("reg.Load(%#v) failed with %v; want success", file, err) + return + } + openapiOptions := &openapiconfig.OpenAPIOptions{ + Service: []*openapiconfig.OpenAPIServiceOption{ + { + Service: "example.ExampleService", + Option: &openapi_options.Tag{ + Description: "ExampleService description", + ExternalDocs: &openapi_options.ExternalDocumentation{ + Description: "Find out more about ExampleService", + }, + }, + }, + }, + } + if err := reg.RegisterOpenAPIOptions(openapiOptions); err != nil { + t.Errorf("reg.RegisterOpenAPIOptions for Service %#v failed with %v; want success", openapiOptions.Service, err) + return + } + + result, err := applyTemplate(param{File: fileCL, reg: reg}) + if err != nil { + t.Errorf("applyTemplate(%#v) failed with %v; want success", file, err) + return + } + if want, is, name := "ExampleService description", result.Tags[0].Description, "Tags[0].Description"; !reflect.DeepEqual(is, want) { + t.Errorf("applyTemplate(%#v).%s = %s want to be %s", file, name, is, want) + } + if want, is, name := "Find out more about ExampleService", result.Tags[0].ExternalDocs.Description, "Tags[0].ExternalDocs.Description"; !reflect.DeepEqual(is, want) { + t.Errorf("applyTemplate(%#v).%s = %s want to be %s", file, name, is, want) + } + + // If there was a failure, print out the input and the json result for debugging. + if t.Failed() { + t.Errorf("had: %s", file) + t.Errorf("got: %s", fmt.Sprint(result)) + } +} + func TestApplyTemplateOverrideOperationID(t *testing.T) { newFile := func() *descriptor.File { msgdesc := &descriptorpb.DescriptorProto{ From 90c1a47248471007ac98a2e76209c9ee10763919 Mon Sep 17 00:00:00 2001 From: y-takuya Date: Fri, 16 Sep 2022 02:36:46 +0900 Subject: [PATCH 3/3] fix: change the config priority for service tags --- protoc-gen-openapiv2/internal/genopenapi/template.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index 518791784e2..0c1fc244c3f 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -2516,17 +2516,13 @@ func getMessageOpenAPIOption(reg *descriptor.Registry, msg *descriptor.Message) } func getServiceOpenAPIOption(reg *descriptor.Registry, svc *descriptor.Service) (*openapi_options.Tag, error) { + if opts, ok := reg.GetOpenAPIServiceOption(svc.FQSN()); ok { + return opts, nil + } opts, err := extractTagOptionFromServiceDescriptor(svc.ServiceDescriptorProto) if err != nil { return nil, err } - if opts != nil { - return opts, nil - } - opts, ok := reg.GetOpenAPIServiceOption(svc.FQSN()) - if !ok { - return nil, nil - } return opts, nil }