From 0f437b68de8421d66da0fc5bfd4eb27acdd9109d Mon Sep 17 00:00:00 2001 From: Maxim Perevedentsev Date: Mon, 24 Apr 2017 20:58:19 +0300 Subject: [PATCH 1/2] Skip unreferenced messages in definitions. If all message fields are embedded in path + query args, the message itself is not referenced, so swagger generates warnings. Here we track request messages referenced in methods, and do not include unreferenced ones. --- examples/clients/abe/Sub2IdMessage.go | 9 ------- .../a_bit_of_everything.swagger.json | 24 ------------------- protoc-gen-swagger/genswagger/template.go | 24 ++++++++++++++----- protoc-gen-swagger/genswagger/types.go | 3 +++ 4 files changed, 21 insertions(+), 39 deletions(-) delete mode 100644 examples/clients/abe/Sub2IdMessage.go diff --git a/examples/clients/abe/Sub2IdMessage.go b/examples/clients/abe/Sub2IdMessage.go deleted file mode 100644 index 77ebe152ba0..00000000000 --- a/examples/clients/abe/Sub2IdMessage.go +++ /dev/null @@ -1,9 +0,0 @@ -package abe - -import ( -) - -type Sub2IdMessage struct { - Uuid string `json:"uuid,omitempty"` - -} diff --git a/examples/examplepb/a_bit_of_everything.swagger.json b/examples/examplepb/a_bit_of_everything.swagger.json index 780d0d9c426..052b256b505 100644 --- a/examples/examplepb/a_bit_of_everything.swagger.json +++ b/examples/examplepb/a_bit_of_everything.swagger.json @@ -718,35 +718,11 @@ "default": "ZERO", "description": "NumericEnum is one or zero.\n\n - ZERO: ZERO means 0\n - ONE: ONE means 1" }, - "protobufDuration": { - "type": "object", - "properties": { - "seconds": { - "type": "string", - "format": "int64", - "description": "Signed seconds of the span of time. Must be from -315,576,000,000\nto +315,576,000,000 inclusive." - }, - "nanos": { - "type": "integer", - "format": "int32", - "description": "Signed fractions of a second at nanosecond resolution of the span\nof time. Durations less than one second are represented with a 0\n`seconds` field and a positive or negative `nanos` field. For durations\nof one second or more, a non-zero value for the `nanos` field must be\nof the same sign as the `seconds` field. Must be from -999,999,999\nto +999,999,999 inclusive." - } - }, - "description": "A Duration represents a signed, fixed-length span of time represented\nas a count of seconds and fractions of seconds at nanosecond\nresolution. It is independent of any calendar and concepts like \"day\"\nor \"month\". It is related to Timestamp in that the difference between\ntwo Timestamp values is a Duration and it can be added or subtracted\nfrom a Timestamp. Range is approximately +-10,000 years.\n\nExample 1: Compute Duration from two Timestamps in pseudo code.\n\n Timestamp start = ...;\n Timestamp end = ...;\n Duration duration = ...;\n\n duration.seconds = end.seconds - start.seconds;\n duration.nanos = end.nanos - start.nanos;\n\n if (duration.seconds \u003c 0 \u0026\u0026 duration.nanos \u003e 0) {\n duration.seconds += 1;\n duration.nanos -= 1000000000;\n } else if (durations.seconds \u003e 0 \u0026\u0026 duration.nanos \u003c 0) {\n duration.seconds -= 1;\n duration.nanos += 1000000000;\n }\n\nExample 2: Compute Timestamp from Timestamp + Duration in pseudo code.\n\n Timestamp start = ...;\n Duration duration = ...;\n Timestamp end = ...;\n\n end.seconds = start.seconds + duration.seconds;\n end.nanos = start.nanos + duration.nanos;\n\n if (end.nanos \u003c 0) {\n end.seconds -= 1;\n end.nanos += 1000000000;\n } else if (end.nanos \u003e= 1000000000) {\n end.seconds += 1;\n end.nanos -= 1000000000;\n }\n\nExample 3: Compute Duration from datetime.timedelta in Python.\n\n td = datetime.timedelta(days=3, minutes=10)\n duration = Duration()\n duration.FromTimedelta(td)" - }, "protobufEmpty": { "type": "object", "description": "service Foo {\n rpc Bar(google.protobuf.Empty) returns (google.protobuf.Empty);\n }\n\nThe JSON representation for `Empty` is empty JSON object `{}`.", "title": "A generic empty message that you can re-use to avoid defining duplicated\nempty messages in your APIs. A typical example is to use it as the request\nor the response type of an API method. For instance:" }, - "sub2IdMessage": { - "type": "object", - "properties": { - "uuid": { - "type": "string" - } - } - }, "subStringMessage": { "type": "object", "properties": { diff --git a/protoc-gen-swagger/genswagger/template.go b/protoc-gen-swagger/genswagger/template.go index 022b649fb9b..170e0e4befd 100644 --- a/protoc-gen-swagger/genswagger/template.go +++ b/protoc-gen-swagger/genswagger/template.go @@ -118,11 +118,14 @@ func queryParams(message *descriptor.Message, field *descriptor.Field, prefix st } // findServicesMessagesAndEnumerations discovers all messages and enums defined in the RPC methods of the service. -func findServicesMessagesAndEnumerations(s []*descriptor.Service, reg *descriptor.Registry, m messageMap, e enumMap) { +func findServicesMessagesAndEnumerations(s []*descriptor.Service, reg *descriptor.Registry, m messageMap, e enumMap, refs refMap) { for _, svc := range s { for _, meth := range svc.Methods { - m[fullyQualifiedNameToSwaggerName(meth.RequestType.FQMN(), reg)] = meth.RequestType - findNestedMessagesAndEnumerations(meth.RequestType, reg, m, e) + // Request may be fully included in query + if _, ok := refs[fmt.Sprintf("#/definitions/%s", fullyQualifiedNameToSwaggerName(meth.RequestType.FQMN(), reg))]; ok { + m[fullyQualifiedNameToSwaggerName(meth.RequestType.FQMN(), reg)] = meth.RequestType + findNestedMessagesAndEnumerations(meth.RequestType, reg, m, e) + } m[fullyQualifiedNameToSwaggerName(meth.ResponseType.FQMN(), reg)] = meth.ResponseType findNestedMessagesAndEnumerations(meth.ResponseType, reg, m, e) } @@ -431,7 +434,7 @@ func templateToSwaggerPath(path string) string { return strings.Join(parts, "/") } -func renderServices(services []*descriptor.Service, paths swaggerPathsObject, reg *descriptor.Registry) error { +func renderServices(services []*descriptor.Service, paths swaggerPathsObject, reg *descriptor.Registry, refs refMap) error { // Correctness of svcIdx and methIdx depends on 'services' containing the services in the same order as the 'file.Service' array. for svcIdx, svc := range services { for methIdx, meth := range svc.Methods { @@ -524,6 +527,14 @@ func renderServices(services []*descriptor.Service, paths swaggerPathsObject, re }, }, } + + // Fill reference map with referenced request messages + for _, param := range operationObject.Parameters { + if param.Schema != nil && param.Schema.Ref != "" { + refs[param.Schema.Ref] = struct{}{} + } + } + methComments := protoComments(reg, svc.File, nil, "Service", int32(svcIdx), methProtoPath, int32(methIdx)) if err := updateSwaggerDataFromComments(operationObject, methComments); err != nil { panic(err) @@ -575,7 +586,8 @@ func applyTemplate(p param) (string, error) { // Loops through all the services and their exposed GET/POST/PUT/DELETE definitions // and create entries for all of them. - if err := renderServices(p.Services, s.Paths, p.reg); err != nil { + refs := refMap{} + if err := renderServices(p.Services, s.Paths, p.reg, refs); err != nil { panic(err) } @@ -583,7 +595,7 @@ func applyTemplate(p param) (string, error) { // write their request and response types out as definition objects. m := messageMap{} e := enumMap{} - findServicesMessagesAndEnumerations(p.Services, p.reg, m, e) + findServicesMessagesAndEnumerations(p.Services, p.reg, m, e, refs) renderMessagesAsDefinition(m, s.Definitions, p.reg) renderEnumerationsAsDefinition(e, s.Definitions, p.reg) diff --git a/protoc-gen-swagger/genswagger/types.go b/protoc-gen-swagger/genswagger/types.go index 3c6a5c9f0de..c328d1c60cc 100644 --- a/protoc-gen-swagger/genswagger/types.go +++ b/protoc-gen-swagger/genswagger/types.go @@ -187,3 +187,6 @@ type messageMap map[string]*descriptor.Message // Internal type mapping from FQEN to descriptor.Enum. Used as a set by the // findServiceMessages function. type enumMap map[string]*descriptor.Enum + +// Internal type to store used references. +type refMap map[string]struct{} From 8f8e790e2d9febf30bf2c783e39db900d67a0a20 Mon Sep 17 00:00:00 2001 From: Maxim Perevedentsev Date: Mon, 8 May 2017 12:46:43 +0300 Subject: [PATCH 2/2] Add a test. --- .../genswagger/template_test.go | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/protoc-gen-swagger/genswagger/template_test.go b/protoc-gen-swagger/genswagger/template_test.go index e0b25c16716..1d438936368 100644 --- a/protoc-gen-swagger/genswagger/template_test.go +++ b/protoc-gen-swagger/genswagger/template_test.go @@ -577,6 +577,133 @@ func TestApplyTemplateRequestWithClientStreaming(t *testing.T) { } } +func TestApplyTemplateRequestWithUnusedReferences(t *testing.T) { + reqdesc := &protodescriptor.DescriptorProto{ + Name: proto.String("ExampleMessage"), + Field: []*protodescriptor.FieldDescriptorProto{ + { + Name: proto.String("string"), + Label: protodescriptor.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Type: protodescriptor.FieldDescriptorProto_TYPE_STRING.Enum(), + Number: proto.Int32(1), + }, + }, + } + respdesc := &protodescriptor.DescriptorProto{ + Name: proto.String("EmptyMessage"), + } + meth := &protodescriptor.MethodDescriptorProto{ + Name: proto.String("Example"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("EmptyMessage"), + ClientStreaming: proto.Bool(false), + ServerStreaming: proto.Bool(false), + } + svc := &protodescriptor.ServiceDescriptorProto{ + Name: proto.String("ExampleService"), + Method: []*protodescriptor.MethodDescriptorProto{meth}, + } + + req := &descriptor.Message{ + DescriptorProto: reqdesc, + } + resp := &descriptor.Message{ + DescriptorProto: respdesc, + } + stringField := &descriptor.Field{ + Message: req, + FieldDescriptorProto: req.GetField()[0], + } + file := descriptor.File{ + FileDescriptorProto: &protodescriptor.FileDescriptorProto{ + SourceCodeInfo: &protodescriptor.SourceCodeInfo{}, + Name: proto.String("example.proto"), + Package: proto.String("example"), + MessageType: []*protodescriptor.DescriptorProto{reqdesc, respdesc}, + Service: []*protodescriptor.ServiceDescriptorProto{svc}, + }, + GoPkg: descriptor.GoPackage{ + Path: "example.com/path/to/example/example.pb", + Name: "example_pb", + }, + Messages: []*descriptor.Message{req, resp}, + Services: []*descriptor.Service{ + { + ServiceDescriptorProto: svc, + Methods: []*descriptor.Method{ + { + MethodDescriptorProto: meth, + RequestType: req, + ResponseType: resp, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/example", + }, + }, + { + HTTPMethod: "POST", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/example/{string}", + }, + PathParams: []descriptor.Parameter{ + { + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{ + { + Name: "string", + Target: stringField, + }, + }), + Target: stringField, + }, + }, + Body: &descriptor.Body{ + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{ + { + Name: "string", + Target: stringField, + }, + }), + }, + }, + }, + }, + }, + }, + }, + } + + reg := descriptor.NewRegistry() + reg.Load(&plugin.CodeGeneratorRequest{ProtoFile: []*protodescriptor.FileDescriptorProto{file.FileDescriptorProto}}) + result, err := applyTemplate(param{File: crossLinkFixture(&file), reg: reg}) + if err != nil { + t.Errorf("applyTemplate(%#v) failed with %v; want success", file, err) + return + } + var obj swaggerObject + err = json.Unmarshal([]byte(result), &obj) + if err != nil { + t.Errorf("applyTemplate(%#v) failed with %v; want success", file, err) + return + } + + // Only EmptyMessage must be present, not ExampleMessage + if want, got, name := 1, len(obj.Definitions), "len(Definitions)"; !reflect.DeepEqual(got, want) { + t.Errorf("applyTemplate(%#v).%s = %d want to be %d", file, name, got, 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", result) + } +} + func TestTemplateToSwaggerPath(t *testing.T) { var tests = []struct { input string