Skip to content

Commit

Permalink
CollectionFormat multi for query params of repeated fields 2 (grpc-ec…
Browse files Browse the repository at this point in the history 
…osystem#909)

* Use collectionFormat multi for query params of repeated fields

Fixes grpc-ecosystem#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 grpc-ecosystem#906.

* deep equal checks in TestSchemaOfField
  • Loading branch information
@bmperrea @johanbrandhorst
bmperrea authored and johanbrandhorst committed Mar 14, 2019
1 parent e81e272 commit 5e6c709
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 65 deletions.
8 changes: 4 additions & 4 deletions examples/clients/abe/a_bit_of_everything_service_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, ""))
Expand Down
12 changes: 8 additions & 4 deletions examples/proto/examplepb/a_bit_of_everything.swagger.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,8 @@
"type": "array",
"items": {
"type": "string"
}
},
"collectionFormat": "multi"
},
{
"name": "oneof_string",
Expand Down Expand Up @@ -361,7 +362,8 @@
"ZERO",
"ONE"
]
}
},
"collectionFormat": "multi"
},
{
"name": "repeated_enum_annotation",
Expand All @@ -375,7 +377,8 @@
"ZERO",
"ONE"
]
}
},
"collectionFormat": "multi"
},
{
"name": "enum_value_annotation",
Expand All @@ -397,7 +400,8 @@
"type": "array",
"items": {
"type": "string"
}
},
"collectionFormat": "multi"
},
{
"name": "nested_annotation.name",
Expand Down
7 changes: 5 additions & 2 deletions protoc-gen-swagger/genswagger/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
Expand Down
107 changes: 59 additions & 48 deletions protoc-gen-swagger/genswagger/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
},
},
Expand All @@ -76,6 +82,13 @@ func TestMessageToQueryParameters(t *testing.T) {
Type: "number",
Format: "double",
},
swaggerParameterObject{
Name: "c",
In: "query",
Required: false,
Type: "array",
CollectionFormat: "multi",
},
},
},
{
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -1006,7 +1023,7 @@ func TestSchemaOfField(t *testing.T) {
refs: make(refMap),
expected: schemaCore{
Type: "string",
Format: "bytes",
Format: "byte",
},
},
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"),
},
}

Expand Down Expand Up @@ -1632,4 +1643,4 @@ func TestUpdateSwaggerDataFromComments(t *testing.T) {
}
})
}
}
}
7 changes: 0 additions & 7 deletions protoc-gen-swagger/genswagger/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 5e6c709

Please sign in to comment.