From 5e6c7098c518ad6773f89a0639fd2285c132e9e1 Mon Sep 17 00:00:00 2001 From: Brent Date: Thu, 14 Mar 2019 09:47:37 -0500 Subject: [PATCH] CollectionFormat multi for query params of repeated fields 2 (#909) * Use collectionFormat multi for query params of repeated fields Fixes #756 . Also formats protoc-gen-swagger/genswagger/template_test.go according to go fmt. * regenerate the files * only specify multi in the method queryParams Fixes #906. * deep equal checks in TestSchemaOfField --- .../abe/a_bit_of_everything_service_api.go | 8 +- .../a_bit_of_everything.swagger.json | 12 +- protoc-gen-swagger/genswagger/template.go | 7 +- .../genswagger/template_test.go | 107 ++++++++++-------- protoc-gen-swagger/genswagger/types.go | 7 -- 5 files changed, 76 insertions(+), 65 deletions(-) diff --git a/examples/clients/abe/a_bit_of_everything_service_api.go b/examples/clients/abe/a_bit_of_everything_service_api.go index ce7c5d817ba..415243210cd 100644 --- a/examples/clients/abe/a_bit_of_everything_service_api.go +++ b/examples/clients/abe/a_bit_of_everything_service_api.go @@ -595,20 +595,20 @@ func (a ABitOfEverythingServiceApi) GetQuery(uuid string, floatValue float32, si localVarQueryParams.Add("sfixed64_value", a.Configuration.APIClient.ParameterToString(sfixed64Value, "")) localVarQueryParams.Add("sint32_value", a.Configuration.APIClient.ParameterToString(sint32Value, "")) localVarQueryParams.Add("sint64_value", a.Configuration.APIClient.ParameterToString(sint64Value, "")) - var repeatedStringValueCollectionFormat = "csv" + var repeatedStringValueCollectionFormat = "multi" localVarQueryParams.Add("repeated_string_value", a.Configuration.APIClient.ParameterToString(repeatedStringValue, repeatedStringValueCollectionFormat)) localVarQueryParams.Add("oneof_string", a.Configuration.APIClient.ParameterToString(oneofString, "")) localVarQueryParams.Add("nonConventionalNameValue", a.Configuration.APIClient.ParameterToString(nonConventionalNameValue, "")) localVarQueryParams.Add("timestamp_value", a.Configuration.APIClient.ParameterToString(timestampValue, "")) - var repeatedEnumValueCollectionFormat = "csv" + var repeatedEnumValueCollectionFormat = "multi" localVarQueryParams.Add("repeated_enum_value", a.Configuration.APIClient.ParameterToString(repeatedEnumValue, repeatedEnumValueCollectionFormat)) - var repeatedEnumAnnotationCollectionFormat = "csv" + var repeatedEnumAnnotationCollectionFormat = "multi" localVarQueryParams.Add("repeated_enum_annotation", a.Configuration.APIClient.ParameterToString(repeatedEnumAnnotation, repeatedEnumAnnotationCollectionFormat)) localVarQueryParams.Add("enum_value_annotation", a.Configuration.APIClient.ParameterToString(enumValueAnnotation, "")) - var repeatedStringAnnotationCollectionFormat = "csv" + var repeatedStringAnnotationCollectionFormat = "multi" localVarQueryParams.Add("repeated_string_annotation", a.Configuration.APIClient.ParameterToString(repeatedStringAnnotation, repeatedStringAnnotationCollectionFormat)) localVarQueryParams.Add("nested_annotation.name", a.Configuration.APIClient.ParameterToString(nestedAnnotationName, "")) diff --git a/examples/proto/examplepb/a_bit_of_everything.swagger.json b/examples/proto/examplepb/a_bit_of_everything.swagger.json index 048fa8da4b3..952baa18492 100644 --- a/examples/proto/examplepb/a_bit_of_everything.swagger.json +++ b/examples/proto/examplepb/a_bit_of_everything.swagger.json @@ -328,7 +328,8 @@ "type": "array", "items": { "type": "string" - } + }, + "collectionFormat": "multi" }, { "name": "oneof_string", @@ -361,7 +362,8 @@ "ZERO", "ONE" ] - } + }, + "collectionFormat": "multi" }, { "name": "repeated_enum_annotation", @@ -375,7 +377,8 @@ "ZERO", "ONE" ] - } + }, + "collectionFormat": "multi" }, { "name": "enum_value_annotation", @@ -397,7 +400,8 @@ "type": "array", "items": { "type": "string" - } + }, + "collectionFormat": "multi" }, { "name": "nested_annotation.name", diff --git a/protoc-gen-swagger/genswagger/template.go b/protoc-gen-swagger/genswagger/template.go index 55f42093b59..b802fef186d 100644 --- a/protoc-gen-swagger/genswagger/template.go +++ b/protoc-gen-swagger/genswagger/template.go @@ -140,6 +140,9 @@ func queryParams(message *descriptor.Message, field *descriptor.Field, prefix st Format: schema.Format, Required: required, } + if param.Type == "array" { + param.CollectionFormat = "multi" + } if reg.GetUseJSONNamesForFields() { param.Name = prefix + field.GetJsonName() @@ -1255,9 +1258,9 @@ func updateSwaggerDataFromComments(swaggerObject interface{}, comment string, is } // overrides the schema value only if it's empty // keep the comment precedence when updating the package definition - if descriptionValue.Len() == 0 || isPackageObject { + if descriptionValue.Len() == 0 || isPackageObject { descriptionValue.Set(reflect.ValueOf(description)) - } + } } return nil } diff --git a/protoc-gen-swagger/genswagger/template_test.go b/protoc-gen-swagger/genswagger/template_test.go index 603e2ed8feb..6f14e5f268b 100644 --- a/protoc-gen-swagger/genswagger/template_test.go +++ b/protoc-gen-swagger/genswagger/template_test.go @@ -58,6 +58,12 @@ func TestMessageToQueryParameters(t *testing.T) { Type: protodescriptor.FieldDescriptorProto_TYPE_DOUBLE.Enum(), Number: proto.Int32(2), }, + { + Name: proto.String("c"), + Type: protodescriptor.FieldDescriptorProto_TYPE_STRING.Enum(), + Label: protodescriptor.FieldDescriptorProto_LABEL_REPEATED.Enum(), + Number: proto.Int32(3), + }, }, }, }, @@ -76,6 +82,13 @@ func TestMessageToQueryParameters(t *testing.T) { Type: "number", Format: "double", }, + swaggerParameterObject{ + Name: "c", + In: "query", + Required: false, + Type: "array", + CollectionFormat: "multi", + }, }, }, { @@ -192,6 +205,10 @@ func TestMessageToQueryParameters(t *testing.T) { if err != nil { t.Fatalf("failed to convert message to query parameters: %s", err) } + // avoid checking Items for array types + for i := range params { + params[i].Items = nil + } if !reflect.DeepEqual(params, test.Params) { t.Errorf("expected %v, got %v", test.Params, params) } @@ -1006,7 +1023,7 @@ func TestSchemaOfField(t *testing.T) { refs: make(refMap), expected: schemaCore{ Type: "string", - Format: "bytes", + Format: "byte", }, }, { @@ -1154,14 +1171,9 @@ func TestSchemaOfField(t *testing.T) { for _, test := range tests { refs := make(refMap) actual := schemaOfField(test.field, reg, refs) - if e, a := test.expected.Type, actual.Type; e != a { - t.Errorf("Expected schemaOfField(%v).Type = %s, actual: %s", test.field, e, a) - } - if e, a := test.expected.Ref, actual.Ref; e != a { - t.Errorf("Expected schemaOfField(%v).Ref = %s, actual: %s", test.field, e, a) - } - if e, a := test.expected.Items.getType(), actual.Items.getType(); e != a { - t.Errorf("Expected schemaOfField(%v).Items.Type = %v, actual.Type: %v", test.field, e, a) + expectedSchemaObject := swaggerSchemaObject{schemaCore: test.expected} + if e, a := expectedSchemaObject, actual; !reflect.DeepEqual(a, e) { + t.Errorf("Expected schemaOfField(%v) = %v, actual: %v", test.field, e, a) } if !reflect.DeepEqual(refs, test.refs) { t.Errorf("Expected schemaOfField(%v) to add refs %v, not %v", test.field, test.refs, refs) @@ -1505,106 +1517,105 @@ func TestProtoComments(t *testing.T) { func TestUpdateSwaggerDataFromComments(t *testing.T) { tests := []struct { - descr string - swaggerObject interface{} - comments string - expectedError error - expectedSwaggerObject interface{} + descr string + swaggerObject interface{} + comments string + expectedError error + expectedSwaggerObject interface{} }{ { - descr: "empty comments", - swaggerObject: nil, + descr: "empty comments", + swaggerObject: nil, expectedSwaggerObject: nil, - comments: "", - expectedError: nil, + comments: "", + expectedError: nil, }, { - descr: "set field to read only", + descr: "set field to read only", swaggerObject: &swaggerSchemaObject{}, expectedSwaggerObject: &swaggerSchemaObject{ - ReadOnly: true, + ReadOnly: true, Description: "... Output only. ...", }, - comments: "... Output only. ...", + comments: "... Output only. ...", expectedError: nil, }, { - descr: "set title", + descr: "set title", swaggerObject: &swaggerSchemaObject{}, expectedSwaggerObject: &swaggerSchemaObject{ Title: "Comment with no trailing dot", }, - comments: "Comment with no trailing dot", + comments: "Comment with no trailing dot", expectedError: nil, }, { - descr: "set description", + descr: "set description", swaggerObject: &swaggerSchemaObject{}, expectedSwaggerObject: &swaggerSchemaObject{ Description: "Comment with trailing dot.", }, - comments: "Comment with trailing dot.", + comments: "Comment with trailing dot.", expectedError: nil, }, { descr: "use info object", swaggerObject: &swaggerObject{ - Info: swaggerInfoObject{ - }, + Info: swaggerInfoObject{}, }, expectedSwaggerObject: &swaggerObject{ Info: swaggerInfoObject{ Description: "Comment with trailing dot.", }, }, - comments: "Comment with trailing dot.", + comments: "Comment with trailing dot.", expectedError: nil, }, { - descr: "multi line comment with title", + descr: "multi line comment with title", swaggerObject: &swaggerSchemaObject{}, - expectedSwaggerObject: &swaggerSchemaObject { - Title: "First line", + expectedSwaggerObject: &swaggerSchemaObject{ + Title: "First line", Description: "Second line", }, - comments: "First line\n\nSecond line", + comments: "First line\n\nSecond line", expectedError: nil, }, { - descr: "multi line comment no title", + descr: "multi line comment no title", swaggerObject: &swaggerSchemaObject{}, - expectedSwaggerObject: &swaggerSchemaObject { + expectedSwaggerObject: &swaggerSchemaObject{ Description: "First line.\n\nSecond line", }, - comments: "First line.\n\nSecond line", + comments: "First line.\n\nSecond line", expectedError: nil, }, { - descr: "multi line comment with summary with dot", + descr: "multi line comment with summary with dot", swaggerObject: &swaggerOperationObject{}, - expectedSwaggerObject: &swaggerOperationObject { - Summary: "First line.", + expectedSwaggerObject: &swaggerOperationObject{ + Summary: "First line.", Description: "Second line", }, - comments: "First line.\n\nSecond line", + comments: "First line.\n\nSecond line", expectedError: nil, }, { - descr: "multi line comment with summary no dot", + descr: "multi line comment with summary no dot", swaggerObject: &swaggerOperationObject{}, - expectedSwaggerObject: &swaggerOperationObject { - Summary: "First line", + expectedSwaggerObject: &swaggerOperationObject{ + Summary: "First line", Description: "Second line", }, - comments: "First line\n\nSecond line", + comments: "First line\n\nSecond line", expectedError: nil, }, { - descr: "multi line comment with summary no dot", - swaggerObject: &schemaCore{}, + descr: "multi line comment with summary no dot", + swaggerObject: &schemaCore{}, expectedSwaggerObject: &schemaCore{}, - comments: "Any comment", - expectedError: errors.New("no description nor summary property"), + comments: "Any comment", + expectedError: errors.New("no description nor summary property"), }, } @@ -1632,4 +1643,4 @@ func TestUpdateSwaggerDataFromComments(t *testing.T) { } }) } -} \ No newline at end of file +} diff --git a/protoc-gen-swagger/genswagger/types.go b/protoc-gen-swagger/genswagger/types.go index 6599937dc69..66aaf177d23 100644 --- a/protoc-gen-swagger/genswagger/types.go +++ b/protoc-gen-swagger/genswagger/types.go @@ -150,13 +150,6 @@ type schemaCore struct { type swaggerItemsObject schemaCore -func (o *swaggerItemsObject) getType() string { - if o == nil { - return "" - } - return o.Type -} - // http://swagger.io/specification/#responsesObject type swaggerResponsesObject map[string]swaggerResponseObject