Skip to content

Commit

Permalink
Issue 11401 - report correctly the parameters with the deep object sp…
Browse files Browse the repository at this point in the history 
…ecification (#13909)

* issue #11401 - Go client generator doesn't support deepObject in query

* samples generation

* fix generation

* fix generation

* generated samples

# Conflicts:
#	samples/client/petstore/go/go-petstore/model_200_response.go
#	samples/client/petstore/go/go-petstore/model_additional_properties_any_type.go
#	samples/client/petstore/go/go-petstore/model_client.go

* Fixed unit tests

* revert to http connection for tests

* fix model_simple generation

* Fix parameter encoding issue

* simplified routine

* fix test url

* adapted for latest master, necessary generation

* samples generation

* sync with new master, regenerate samples

* added api client test
  • Loading branch information
@parvit
parvit authored Nov 20, 2022
1 parent 95b566a commit 4487042
Show file tree
Hide file tree
Showing 124 changed files with 2,103 additions and 445 deletions.
22 changes: 11 additions & 11 deletions modules/openapi-generator/src/main/resources/go/api.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (a *{{{classname}}}Service) {{nickname}}Execute(r {{#structPrefix}}{{&class
}

localVarPath := localBasePath + "{{{path}}}"{{#pathParams}}
localVarPath = strings.Replace(localVarPath, "{"+"{{baseName}}"+"}", url.PathEscape(parameterToString(r.{{paramName}}, "{{collectionFormat}}")), -1){{/pathParams}}
localVarPath = strings.Replace(localVarPath, "{"+"{{baseName}}"+"}", url.PathEscape(parameterValueToString(r.{{paramName}}, "{{paramName}}")), -1){{/pathParams}}

localVarHeaderParams := make(map[string]string)
localVarQueryParams := url.Values{}
Expand Down Expand Up @@ -189,15 +189,15 @@ func (a *{{{classname}}}Service) {{nickname}}Execute(r {{#structPrefix}}{{&class
if reflect.TypeOf(t).Kind() == reflect.Slice {
s := reflect.ValueOf(t)
for i := 0; i < s.Len(); i++ {
localVarQueryParams.Add("{{baseName}}", parameterToString(s.Index(i), "{{collectionFormat}}"))
parameterAddToQuery(localVarQueryParams, "{{baseName}}", s.Index(i), "{{collectionFormat}}")
}
} else {
localVarQueryParams.Add("{{baseName}}", parameterToString(t, "{{collectionFormat}}"))
parameterAddToQuery(localVarQueryParams, "{{baseName}}", t, "{{collectionFormat}}")
}
}
{{/isCollectionFormatMulti}}
{{^isCollectionFormatMulti}}
localVarQueryParams.Add("{{baseName}}", parameterToString(*r.{{paramName}}, "{{collectionFormat}}"))
parameterAddToQuery(localVarQueryParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")
{{/isCollectionFormatMulti}}
{{/required}}
{{^required}}
Expand All @@ -207,14 +207,14 @@ func (a *{{{classname}}}Service) {{nickname}}Execute(r {{#structPrefix}}{{&class
if reflect.TypeOf(t).Kind() == reflect.Slice {
s := reflect.ValueOf(t)
for i := 0; i < s.Len(); i++ {
localVarQueryParams.Add("{{baseName}}", parameterToString(s.Index(i), "{{collectionFormat}}"))
parameterAddToQuery(localVarQueryParams, "{{baseName}}", s.Index(i), "{{collectionFormat}}")
}
} else {
localVarQueryParams.Add("{{baseName}}", parameterToString(t, "{{collectionFormat}}"))
parameterAddToQuery(localVarQueryParams, "{{baseName}}", t, "{{collectionFormat}}")
}
{{/isCollectionFormatMulti}}
{{^isCollectionFormatMulti}}
localVarQueryParams.Add("{{baseName}}", parameterToString(*r.{{paramName}}, "{{collectionFormat}}"))
parameterAddToQuery(localVarQueryParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")
{{/isCollectionFormatMulti}}
}
{{/required}}
Expand Down Expand Up @@ -242,11 +242,11 @@ func (a *{{{classname}}}Service) {{nickname}}Execute(r {{#structPrefix}}{{&class
}
{{#headerParams}}
{{#required}}
localVarHeaderParams["{{baseName}}"] = parameterToString(*r.{{paramName}}, "{{collectionFormat}}")
parameterAddToQuery(localVarQueryParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")
{{/required}}
{{^required}}
if r.{{paramName}} != nil {
localVarHeaderParams["{{baseName}}"] = parameterToString(*r.{{paramName}}, "{{collectionFormat}}")
parameterAddToQuery(localVarQueryParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")
}
{{/required}}
{{/headerParams}}
Expand Down Expand Up @@ -277,7 +277,7 @@ func (a *{{{classname}}}Service) {{nickname}}Execute(r {{#structPrefix}}{{&class
{{/isFile}}
{{^isFile}}
{{#required}}
localVarFormParams.Add("{{baseName}}", parameterToString(*r.{{paramName}}, "{{collectionFormat}}"))
parameterAddToQuery(localVarFormParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")
{{/required}}
{{^required}}
{{#isModel}}
Expand All @@ -291,7 +291,7 @@ func (a *{{{classname}}}Service) {{nickname}}Execute(r {{#structPrefix}}{{&class
{{/isModel}}
{{^isModel}}
if r.{{paramName}} != nil {
localVarFormParams.Add("{{baseName}}", parameterToString(*r.{{paramName}}, "{{collectionFormat}}"))
parameterAddToQuery(localVarFormParams, "{{baseName}}", r.{{paramName}}, "{{collectionFormat}}")
}
{{/isModel}}
{{/required}}
Expand Down
121 changes: 100 additions & 21 deletions modules/openapi-generator/src/main/resources/go/client.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import (
var (
jsonCheck = regexp.MustCompile(`(?i:(?:application|text)/(?:vnd\.[^;]+\+)?json)`)
xmlCheck = regexp.MustCompile(`(?i:(?:application|text)/xml)`)
queryParamSplit = regexp.MustCompile(`(^|&)([^&]+)`)
queryDescape = strings.NewReplacer( "%5B", "[", "%5D", "]" )
)

// APIClient manages communication with the {{appName}} API v{{version}}
Expand Down Expand Up @@ -132,28 +134,101 @@ func typeCheckParameter(obj interface{}, expected string, name string) error {
return nil
}

// parameterToString convert interface{} parameters to string, using a delimiter if format is provided.
func parameterToString(obj interface{}, collectionFormat string) string {
var delimiter string
switch collectionFormat {
case "pipes":
delimiter = "|"
case "ssv":
delimiter = " "
case "tsv":
delimiter = "\t"
case "csv":
delimiter = ","
}
func parameterValueToString( obj interface{}, key string ) string {
if reflect.TypeOf(obj).Kind() != reflect.Ptr {
return fmt.Sprintf("%v", obj)
}
var param,ok = obj.(MappedNullable)
if !ok {
return ""
}
dataMap,err := param.ToMap()
if err != nil {
return ""
}
return fmt.Sprintf("%v", dataMap[key])
}

if reflect.TypeOf(obj).Kind() == reflect.Slice {
return strings.Trim(strings.Replace(fmt.Sprint(obj), " ", delimiter, -1), "[]")
} else if t, ok := obj.(time.Time); ok {
return t.Format(time.RFC3339)
}
// parameterAddToQuery adds the provided object to the url query supporting deep object syntax
func parameterAddToQuery(queryParams interface{}, keyPrefix string, obj interface{}, collectionType string) {
var v = reflect.ValueOf(obj)
var value = ""
if v == reflect.ValueOf(nil) {
value = "null"
} else {
switch v.Kind() {
case reflect.Invalid:
value = "invalid"
case reflect.Struct:
if t,ok := obj.(MappedNullable); ok {
dataMap,err := t.ToMap()
if err != nil {
return
}
parameterAddToQuery(queryParams, keyPrefix, dataMap, collectionType)
return
}
if t, ok := obj.(time.Time); ok {
parameterAddToQuery(queryParams, keyPrefix, t.Format(time.RFC3339), collectionType)
return
}
value = v.Type().String() + " value"
case reflect.Slice:
var indValue = reflect.ValueOf(obj)
if indValue == reflect.ValueOf(nil) {
return
}
var lenIndValue = indValue.Len()
for i:=0;i<lenIndValue;i++ {
var arrayValue = indValue.Index(i)
parameterAddToQuery(queryParams, keyPrefix, arrayValue.Interface(), collectionType)
}
return

return fmt.Sprintf("%v", obj)
case reflect.Map:
var indValue = reflect.ValueOf(obj)
if indValue == reflect.ValueOf(nil) {
return
}
iter := indValue.MapRange()
for iter.Next() {
k,v := iter.Key(), iter.Value()
parameterAddToQuery(queryParams, fmt.Sprintf("%s[%s]", keyPrefix, k.String()), v.Interface(), collectionType)
}
return

case reflect.Interface:
fallthrough
case reflect.Ptr:
parameterAddToQuery(queryParams, keyPrefix, v.Elem().Interface(), collectionType)
return

case reflect.Int, reflect.Int8, reflect.Int16,
reflect.Int32, reflect.Int64:
value = strconv.FormatInt(v.Int(), 10)
case reflect.Uint, reflect.Uint8, reflect.Uint16,
reflect.Uint32, reflect.Uint64, reflect.Uintptr:
value = strconv.FormatUint(v.Uint(), 10)
case reflect.Float32, reflect.Float64:
value = strconv.FormatFloat(v.Float(), 'g', -1, 32)
case reflect.Bool:
value = strconv.FormatBool(v.Bool())
case reflect.String:
value = v.String()
default:
value = v.Type().String() + " value"
}
}

switch valuesMap := queryParams.(type) {
case url.Values:
valuesMap.Add( keyPrefix, value )
break
case map[string]string:
valuesMap[keyPrefix] = value
break
}
}

// helper for converting interface{} parameters to json strings
Expand Down Expand Up @@ -305,7 +380,11 @@ func (c *APIClient) prepareRequest(
}
// Encode the parameters.
url.RawQuery = query.Encode()
url.RawQuery = queryParamSplit.ReplaceAllStringFunc(query.Encode(), func(s string) string {
pieces := strings.Split(s, "=")
pieces[0] = queryDescape.Replace(pieces[0])
return strings.Join(pieces, "=")
})
// Generate a new request
if body != nil {
Expand Down
47 changes: 34 additions & 13 deletions modules/openapi-generator/src/main/resources/go/model_simple.mustache
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// checks if the {{classname}} type satisfies the MappedNullable interface at compile time
var _ MappedNullable = &{{classname}}{}

// {{classname}} {{{description}}}{{^description}}struct for {{{classname}}}{{/description}}
type {{classname}} struct {
{{#parent}}
Expand Down Expand Up @@ -123,12 +126,12 @@ func (o *{{classname}}) Get{{name}}() {{vendorExtensions.x-go-base-type}} {
{{/deprecated}}
func (o *{{classname}}) Get{{name}}Ok() ({{^isArray}}{{^isFreeFormObject}}*{{/isFreeFormObject}}{{/isArray}}{{vendorExtensions.x-go-base-type}}, bool) {
if o == nil{{#isNullable}}{{#vendorExtensions.x-golang-is-container}} || isNil(o.{{name}}){{/vendorExtensions.x-golang-is-container}}{{/isNullable}} {
{{^isFreeFormObject}}
{{^isFreeFormObject}}
return nil, false
{{/isFreeFormObject}}
{{#isFreeFormObject}}
{{/isFreeFormObject}}
{{#isFreeFormObject}}
return {{vendorExtensions.x-go-base-type}}{}, false
{{/isFreeFormObject}}
{{/isFreeFormObject}}
}
{{#isNullable}}
{{#vendorExtensions.x-golang-is-container}}
Expand Down Expand Up @@ -195,12 +198,12 @@ func (o *{{classname}}) Get{{name}}() {{vendorExtensions.x-go-base-type}} {
{{/deprecated}}
func (o *{{classname}}) Get{{name}}Ok() ({{^isArray}}{{^isFreeFormObject}}*{{/isFreeFormObject}}{{/isArray}}{{vendorExtensions.x-go-base-type}}, bool) {
if o == nil{{^isNullable}} || isNil(o.{{name}}){{/isNullable}}{{#isNullable}}{{#vendorExtensions.x-golang-is-container}} || isNil(o.{{name}}){{/vendorExtensions.x-golang-is-container}}{{/isNullable}} {
{{^isFreeFormObject}}
{{^isFreeFormObject}}
return nil, false
{{/isFreeFormObject}}
{{#isFreeFormObject}}
{{/isFreeFormObject}}
{{#isFreeFormObject}}
return {{vendorExtensions.x-go-base-type}}{}, false
{{/isFreeFormObject}}
{{/isFreeFormObject}}
}
{{#isNullable}}
{{#vendorExtensions.x-golang-is-container}}
Expand Down Expand Up @@ -258,17 +261,25 @@ func (o *{{classname}}) Unset{{name}}() {
{{/required}}
{{/vars}}
func (o {{classname}}) MarshalJSON() ([]byte, error) {
toSerialize,err := o.ToMap()
if err != nil {
return []byte{}, err
}
return json.Marshal(toSerialize)
}

func (o {{classname}}) ToMap() (map[string]interface{}, error) {
toSerialize := {{#isArray}}make([]interface{}, len(o.Items)){{/isArray}}{{^isArray}}map[string]interface{}{}{{/isArray}}
{{#parent}}
{{^isMap}}
{{^isArray}}
serialized{{parent}}, err{{parent}} := json.Marshal(o.{{parent}})
if err{{parent}} != nil {
return []byte{}, err{{parent}}
return map[string]interface{}{}, err{{parent}}
}
err{{parent}} = json.Unmarshal([]byte(serialized{{parent}}), &toSerialize)
if err{{parent}} != nil {
return []byte{}, err{{parent}}
return map[string]interface{}{}, err{{parent}}
}
{{/isArray}}
{{/isMap}}
Expand All @@ -288,16 +299,26 @@ func (o {{classname}}) MarshalJSON() ([]byte, error) {
}
{{/vendorExtensions.x-golang-is-container}}
{{^vendorExtensions.x-golang-is-container}}
if {{#required}}true{{/required}}{{^required}}o.{{name}}.IsSet(){{/required}} {
{{#required}}
toSerialize["{{baseName}}"] = o.{{name}}.Get()
{{/required}}
{{^required}}
if o.{{name}}.IsSet() {
toSerialize["{{baseName}}"] = o.{{name}}.Get()
}
{{/required}}
{{/vendorExtensions.x-golang-is-container}}
{{/isNullable}}
{{! if argument is not nullable, don't set it if it is nil}}
{{^isNullable}}
if {{#required}}true{{/required}}{{^required}}!isNil(o.{{name}}){{/required}} {
{{#required}}
toSerialize["{{baseName}}"] = o.{{name}}
{{/required}}
{{^required}}
if !isNil(o.{{name}}) {
toSerialize["{{baseName}}"] = o.{{name}}
}
{{/required}}
{{/isNullable}}
{{/vars}}
{{#isAdditionalPropertiesTrue}}
Expand All @@ -307,7 +328,7 @@ func (o {{classname}}) MarshalJSON() ([]byte, error) {
}
{{/isAdditionalPropertiesTrue}}
return json.Marshal(toSerialize)
return toSerialize, nil
}
{{#isAdditionalPropertiesTrue}}
Expand Down
4 changes: 4 additions & 0 deletions modules/openapi-generator/src/main/resources/go/utils.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,3 +332,7 @@ func isNil(i interface{}) bool {
}
return false
}

type MappedNullable interface {
ToMap() (map[string]interface{}, error)
}
25 changes: 24 additions & 1 deletion ...resources/3_0/go/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ paths:
'5XX':
description: Something wrong
content:
'applicatino/json':
'application/json':
schema:
$ref: '#/components/schemas/OuterNumber'
/fake/outer/number:
Expand Down Expand Up @@ -1155,6 +1155,29 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/HealthCheckResult'
/fake/deep_object_test:
get:
tags:
- fake
operationId: test_query_deep_object
parameters:
- name: test_pet
in: query
required: false
style: deepObject
schema:
$ref: '#/components/schemas/Pet'
explode: true
- name: inputOptions
in: query
required: false
style: deepObject
schema:
$ref: '#/components/schemas/Category'
explode: true
responses:
'200':
description: OK
servers:
- url: 'http://{server}.swagger.io:{port}/v2'
description: petstore server
Expand Down
Loading

1 comment on commit 4487042

@barret0224
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I observed that, this change is actually introducing a compilation error generated from the model_simple.mustache.

The ToMap function is not handling properly the case when the model isArray

func (o {{classname}}) ToMap() (map[string]interface{}, error) {
	toSerialize := {{#isArray}}make([]interface{}, len(o.Items)){{/isArray}}{{^isArray}}map[string]interface{}{}{{/isArray}}

...
	return toSerialize

here the ToMap function is returning a map[string]interface{} but the toSerialize is a []interface{} in case the model is array.

Previously this was part of the MarshalJSON function, now this new MappedNullable interface is introduced for some reason, so the ToMap was created.

Please sign in to comment.