From 25f40c4a03e76e1ed15004e1d4eb719e2a2ac078 Mon Sep 17 00:00:00 2001 From: Brent Date: Sat, 9 Mar 2019 23:11:59 -0600 Subject: [PATCH 1/2] Use collectionFormat multi for query params of repeated fields Solves #756 . Also formats protoc-gen-swagger/genswagger/template_test.go according to go fmt. --- protoc-gen-swagger/genswagger/template.go | 20 ++-- .../genswagger/template_test.go | 94 +++++++++++-------- protoc-gen-swagger/genswagger/types.go | 1 + 3 files changed, 67 insertions(+), 48 deletions(-) diff --git a/protoc-gen-swagger/genswagger/template.go b/protoc-gen-swagger/genswagger/template.go index 55f42093b59..b4f1b9b8460 100644 --- a/protoc-gen-swagger/genswagger/template.go +++ b/protoc-gen-swagger/genswagger/template.go @@ -132,13 +132,14 @@ func queryParams(message *descriptor.Message, field *descriptor.Field, prefix st } param := swaggerParameterObject{ - Description: desc, - In: "query", - Default: schema.Default, - Type: schema.Type, - Items: schema.Items, - Format: schema.Format, - Required: required, + Description: desc, + In: "query", + Default: schema.Default, + Type: schema.Type, + Items: schema.Items, + Format: schema.Format, + CollectionFormat: schema.CollectionFormat, + Required: required, } if reg.GetUseJSONNamesForFields() { @@ -423,6 +424,7 @@ func schemaOfField(f *descriptor.Field, reg *descriptor.Registry, refs refMap) s Type: "array", Items: (*swaggerItemsObject)(&core), }, + CollectionFormat: "multi", } case object: ret = swaggerSchemaObject{ @@ -1255,9 +1257,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..d2e4c05b61f 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) } @@ -1505,106 +1522,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 +1648,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..ddc63167db1 100644 --- a/protoc-gen-swagger/genswagger/types.go +++ b/protoc-gen-swagger/genswagger/types.go @@ -224,6 +224,7 @@ type swaggerSchemaObject struct { MaxProperties uint64 `json:"maxProperties,omitempty"` MinProperties uint64 `json:"minProperties,omitempty"` Required []string `json:"required,omitempty"` + CollectionFormat string `json:"collectionFormat,omitempty"` } // http://swagger.io/specification/#referenceObject From 30a13e5b574c23d7466688ea42a286d366c5e80c Mon Sep 17 00:00:00 2001 From: Brent Date: Sun, 10 Mar 2019 11:41:59 -0500 Subject: [PATCH 2/2] regenerate the files --- .../abe/a_bit_of_everything_service_api.go | 8 +- .../a_bit_of_everything.swagger.json | 81 ++++++++++++------- .../response_body_service.swagger.json | 12 ++- examples/proto/examplepb/stream.swagger.json | 21 +++-- 4 files changed, 80 insertions(+), 42 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..0bbb58038e2 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", @@ -1526,7 +1530,8 @@ "type": "array", "items": { "$ref": "#/definitions/ABitOfEverythingNested" - } + }, + "collectionFormat": "multi" }, "float_value": { "type": "number", @@ -1605,7 +1610,8 @@ "type": "array", "items": { "type": "string" - } + }, + "collectionFormat": "multi" }, "oneof_empty": { "properties": {} @@ -1643,7 +1649,8 @@ "items": { "$ref": "#/definitions/examplepbNumericEnum" }, - "title": "repeated enum value. it is comma-separated in query" + "title": "repeated enum value. it is comma-separated in query", + "collectionFormat": "multi" }, "repeated_enum_annotation": { "type": "array", @@ -1651,7 +1658,8 @@ "$ref": "#/definitions/examplepbNumericEnum" }, "description": "Repeated numeric enum description.", - "title": "Repeated numeric enum title" + "title": "Repeated numeric enum title", + "collectionFormat": "multi" }, "enum_value_annotation": { "$ref": "#/definitions/examplepbNumericEnum", @@ -1664,7 +1672,8 @@ "type": "string" }, "description": "Repeated string description.", - "title": "Repeated string title" + "title": "Repeated string title", + "collectionFormat": "multi" }, "repeated_nested_annotation": { "type": "array", @@ -1672,7 +1681,8 @@ "$ref": "#/definitions/ABitOfEverythingNested" }, "description": "Repeated nested object description.", - "title": "Repeated nested object title" + "title": "Repeated nested object title", + "collectionFormat": "multi" }, "nested_annotation": { "$ref": "#/definitions/ABitOfEverythingNested", @@ -1714,110 +1724,126 @@ "type": "number", "format": "float" }, - "title": "repeated values. they are comma-separated in path" + "title": "repeated values. they are comma-separated in path", + "collectionFormat": "multi" }, "path_repeated_double_value": { "type": "array", "items": { "type": "number", "format": "double" - } + }, + "collectionFormat": "multi" }, "path_repeated_int64_value": { "type": "array", "items": { "type": "string", "format": "int64" - } + }, + "collectionFormat": "multi" }, "path_repeated_uint64_value": { "type": "array", "items": { "type": "string", "format": "uint64" - } + }, + "collectionFormat": "multi" }, "path_repeated_int32_value": { "type": "array", "items": { "type": "integer", "format": "int32" - } + }, + "collectionFormat": "multi" }, "path_repeated_fixed64_value": { "type": "array", "items": { "type": "string", "format": "uint64" - } + }, + "collectionFormat": "multi" }, "path_repeated_fixed32_value": { "type": "array", "items": { "type": "integer", "format": "int64" - } + }, + "collectionFormat": "multi" }, "path_repeated_bool_value": { "type": "array", "items": { "type": "boolean", "format": "boolean" - } + }, + "collectionFormat": "multi" }, "path_repeated_string_value": { "type": "array", "items": { "type": "string" - } + }, + "collectionFormat": "multi" }, "path_repeated_bytes_value": { "type": "array", "items": { "type": "string", "format": "byte" - } + }, + "collectionFormat": "multi" }, "path_repeated_uint32_value": { "type": "array", "items": { "type": "integer", "format": "int64" - } + }, + "collectionFormat": "multi" }, "path_repeated_enum_value": { "type": "array", "items": { "$ref": "#/definitions/examplepbNumericEnum" - } + }, + "collectionFormat": "multi" }, "path_repeated_sfixed32_value": { "type": "array", "items": { "type": "integer", "format": "int32" - } + }, + "collectionFormat": "multi" }, "path_repeated_sfixed64_value": { "type": "array", "items": { "type": "string", "format": "int64" - } + }, + "collectionFormat": "multi" }, "path_repeated_sint32_value": { "type": "array", "items": { "type": "integer", "format": "int32" - } + }, + "collectionFormat": "multi" }, "path_repeated_sint64_value": { "type": "array", "items": { "type": "string", "format": "int64" - } + }, + "collectionFormat": "multi" } }, "title": "ABitOfEverythingRepeated is used to validate repeated path parameter functionality" @@ -1867,7 +1893,8 @@ "items": { "type": "string" }, - "description": "The set of field mask paths." + "description": "The set of field mask paths.", + "collectionFormat": "multi" } }, "description": "paths: \"f.a\"\n paths: \"f.b.d\"\n\nHere `f` represents a field in some root message, `a` and `b`\nfields in the message found in `f`, and `d` a field found in the\nmessage in `f.b`.\n\nField masks are used to specify a subset of fields that should be\nreturned by a get operation or modified by an update operation.\nField masks also have a custom JSON encoding (see below).\n\n# Field Masks in Projections\n\nWhen used in the context of a projection, a response message or\nsub-message is filtered by the API to only contain those fields as\nspecified in the mask. For example, if the mask in the previous\nexample is applied to a response message as follows:\n\n f {\n a : 22\n b {\n d : 1\n x : 2\n }\n y : 13\n }\n z: 8\n\nThe result will not contain specific values for fields x,y and z\n(their value will be set to the default, and omitted in proto text\noutput):\n\n\n f {\n a : 22\n b {\n d : 1\n }\n }\n\nA repeated field is not allowed except at the last position of a\npaths string.\n\nIf a FieldMask object is not present in a get operation, the\noperation applies to all fields (as if a FieldMask of all fields\nhad been specified).\n\nNote that a field mask does not necessarily apply to the\ntop-level response message. In case of a REST get operation, the\nfield mask applies directly to the response, but in case of a REST\nlist operation, the mask instead applies to each individual message\nin the returned resource list. In case of a REST custom method,\nother definitions may be used. Where the mask applies will be\nclearly documented together with its declaration in the API. In\nany case, the effect on the returned resource/resources is required\nbehavior for APIs.\n\n# Field Masks in Update Operations\n\nA field mask in update operations specifies which fields of the\ntargeted resource are going to be updated. The API is required\nto only change the values of the fields as specified in the mask\nand leave the others untouched. If a resource is passed in to\ndescribe the updated values, the API ignores the values of all\nfields not covered by the mask.\n\nIf a repeated field is specified for an update operation, new values will\nbe appended to the existing repeated field in the target resource. Note that\na repeated field is only allowed in the last position of a `paths` string.\n\nIf a sub-message is specified in the last position of the field mask for an\nupdate operation, then new value will be merged into the existing sub-message\nin the target resource.\n\nFor example, given the target message:\n\n f {\n b {\n d: 1\n x: 2\n }\n c: [1]\n }\n\nAnd an update message:\n\n f {\n b {\n d: 10\n }\n c: [2]\n }\n\nthen if the field mask is:\n\n paths: [\"f.b\", \"f.c\"]\n\nthen the result will be:\n\n f {\n b {\n d: 10\n x: 2\n }\n c: [1, 2]\n }\n\nAn implementation may provide options to override this default behavior for\nrepeated and message fields.\n\nIn order to reset a field's value to the default, the field must\nbe in the mask and set to the default value in the provided resource.\nHence, in order to reset all fields of a resource, provide a default\ninstance of the resource and set all fields in the mask, or do\nnot provide a mask as described below.\n\nIf a field mask is not present on update, the operation applies to\nall fields (as if a field mask of all fields has been specified).\nNote that in the presence of schema evolution, this may mean that\nfields the client does not know and has therefore not filled into\nthe request will be reset to their default. If this is unwanted\nbehavior, a specific service may require a client to always specify\na field mask, producing an error if not.\n\nAs with get operations, the location of the resource which\ndescribes the updated values in the request message depends on the\noperation kind. In any case, the effect of the field mask is\nrequired to be honored by the API.\n\n## Considerations for HTTP REST\n\nThe HTTP kind of an update operation which uses a field mask must\nbe set to PATCH instead of PUT in order to satisfy HTTP semantics\n(PUT must only be used for full updates).\n\n# JSON Encoding of Field Masks\n\nIn JSON, a field mask is encoded as a single string where paths are\nseparated by a comma. Fields name in each path are converted\nto/from lower-camel naming conventions.\n\nAs an example, consider the following message declarations:\n\n message Profile {\n User user = 1;\n Photo photo = 2;\n }\n message User {\n string display_name = 1;\n string address = 2;\n }\n\nIn proto a field mask for `Profile` may look as such:\n\n mask {\n paths: \"user.display_name\"\n paths: \"photo\"\n }\n\nIn JSON, the same mask is represented as below:\n\n {\n mask: \"user.displayName,photo\"\n }\n\n# Field Masks and Oneof Fields\n\nField masks treat fields in oneofs just as regular fields. Consider the\nfollowing message:\n\n message SampleMessage {\n oneof test_oneof {\n string name = 4;\n SubMessage sub_message = 9;\n }\n }\n\nThe field mask can be:\n\n mask {\n paths: \"name\"\n }\n\nOr:\n\n mask {\n paths: \"sub_message\"\n }\n\nNote that oneof type names (\"test_oneof\" in this case) cannot be used in\npaths.\n\n## Field Mask Verification\n\nThe implementation of any API method which has a FieldMask type field in the\nrequest should verify the included field paths, and return an\n`INVALID_ARGUMENT` error if any path is duplicated or unmappable.", diff --git a/examples/proto/examplepb/response_body_service.swagger.json b/examples/proto/examplepb/response_body_service.swagger.json index 820129e93eb..3150845833c 100644 --- a/examples/proto/examplepb/response_body_service.swagger.json +++ b/examples/proto/examplepb/response_body_service.swagger.json @@ -25,7 +25,8 @@ "type": "array", "items": { "$ref": "#/definitions/examplepbRepeatedResponseBodyOutResponse" - } + }, + "collectionFormat": "multi" } } }, @@ -76,7 +77,8 @@ "type": "array", "items": { "type": "string" - } + }, + "collectionFormat": "multi" } } }, @@ -112,7 +114,8 @@ "type": "array", "items": { "$ref": "#/definitions/examplepbRepeatedResponseBodyOutResponse" - } + }, + "collectionFormat": "multi" } } }, @@ -134,7 +137,8 @@ "type": "array", "items": { "type": "string" - } + }, + "collectionFormat": "multi" } } }, diff --git a/examples/proto/examplepb/stream.swagger.json b/examples/proto/examplepb/stream.swagger.json index d67b6d24496..e24bd55feb6 100644 --- a/examples/proto/examplepb/stream.swagger.json +++ b/examples/proto/examplepb/stream.swagger.json @@ -143,7 +143,8 @@ "type": "array", "items": { "$ref": "#/definitions/ABitOfEverythingNested" - } + }, + "collectionFormat": "multi" }, "float_value": { "type": "number", @@ -222,7 +223,8 @@ "type": "array", "items": { "type": "string" - } + }, + "collectionFormat": "multi" }, "oneof_empty": { "properties": {} @@ -260,7 +262,8 @@ "items": { "$ref": "#/definitions/examplepbNumericEnum" }, - "title": "repeated enum value. it is comma-separated in query" + "title": "repeated enum value. it is comma-separated in query", + "collectionFormat": "multi" }, "repeated_enum_annotation": { "type": "array", @@ -268,7 +271,8 @@ "$ref": "#/definitions/examplepbNumericEnum" }, "description": "Repeated numeric enum description.", - "title": "Repeated numeric enum title" + "title": "Repeated numeric enum title", + "collectionFormat": "multi" }, "enum_value_annotation": { "$ref": "#/definitions/examplepbNumericEnum", @@ -281,7 +285,8 @@ "type": "string" }, "description": "Repeated string description.", - "title": "Repeated string title" + "title": "Repeated string title", + "collectionFormat": "multi" }, "repeated_nested_annotation": { "type": "array", @@ -289,7 +294,8 @@ "$ref": "#/definitions/ABitOfEverythingNested" }, "description": "Repeated nested object description.", - "title": "Repeated nested object title" + "title": "Repeated nested object title", + "collectionFormat": "multi" }, "nested_annotation": { "$ref": "#/definitions/ABitOfEverythingNested", @@ -362,7 +368,8 @@ "type": "array", "items": { "$ref": "#/definitions/protobufAny" - } + }, + "collectionFormat": "multi" } } },