Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: support repeated query params and simplified "csv" cases #304

Merged
merged 2 commits into from
Sep 17, 2020
Merged

feat: support repeated query params and simplified "csv" cases #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39871ea
feat: support repeated query params and simplified "csv" cases
zaquestion Aug 20, 2020
5891a72
update client tmpl to use new repeated query param strategy
zaquestion Sep 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 25 additions & 31 deletions cmd/_integration-tests/transport/http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,16 @@ func TestGetWithRepeatedQueryRequest(t *testing.T) {
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
}

// csv style
err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedquery?%s=%d,%d", "A", A[0], A[1])
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
}
// multi / golang style
// err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedquery?%s=%d&%s=%d", "A", A[0], "A", A[1])
// if err != nil {
// t.Fatal(errors.Wrap(err, "cannot make http request"))
// }
err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedquery?%s=%d&%s=%d", "A", A[0], "A", A[1])
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
}
}

func TestGetWithRepeatedStringQueryClient(t *testing.T) {
Expand Down Expand Up @@ -146,37 +145,32 @@ func TestGetWithRepeatedStringQueryRequest(t *testing.T) {
expects := pb.GetWithRepeatedStringQueryResponse{
V: A[0] + A[1],
}

var err error

err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedstringquery?%s=[\"%s\",\"%s\"]", "A", A[0], A[1])
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
expectsSingle := pb.GetWithRepeatedStringQueryResponse{
V: A[0],
}

// csv style
err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedstringquery?%s=\"%s\",\"%s\"", "A", A[0], A[1])
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
}
var err error

// default array, no quotes
// err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedstringquery?%s=[%s,%s]", "A", A[0], A[1])
// if err != nil {
// t.Fatal(errors.Wrap(err, "cannot make http request"))
// }
t.Run("single Value", func(t *testing.T) {
err = testHTTP(t, &resp, &expectsSingle, nil, "GET", "getwithrepeatedstringquery?%s=%s", "A", A[0])
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
}
})

// csv style, no quotes
// err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedstringquery?%s=[%s,%s]", "A", A[0], A[1])
// if err != nil {
// t.Fatal(errors.Wrap(err, "cannot make http request"))
// }
t.Run("csv style, no quotes", func(t *testing.T) {
err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedstringquery?%s=%s,%s", "A", A[0], A[1])
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
}
})

// multi / golang style
// err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedstringquery?%s=%s&%s=%s", "A", A[0], "A", A[1])
// if err != nil {
// t.Fatal(errors.Wrap(err, "cannot make http request"))
// }
t.Run("multi / golang style", func(t *testing.T) {
err = testHTTP(t, &resp, &expects, nil, "GET", "getwithrepeatedstringquery?%s=%s&%s=%s", "A", A[0], "A", A[1])
if err != nil {
t.Fatal(errors.Wrap(err, "cannot make http request"))
}
})

}

Expand Down
67 changes: 50 additions & 17 deletions gengokit/httptransport/httptransport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"text/template"
"unicode"

log "github.com/sirupsen/logrus"
gogen "github.com/gogo/protobuf/protoc-gen-gogo/generator"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"

"github.com/metaverse/truss/gengokit/httptransport/templates"
"github.com/metaverse/truss/svcdef"
Expand Down Expand Up @@ -345,8 +345,12 @@ case {{$option.LocalName}}OK:
// convert the string form of the field to it's correct go type.
func createDecodeConvertFunc(f Field) (string, bool) {
needsErrorCheck := true
// We may leverage the below convert logic for repeated base types. By
// triming the slice prefix we can easily store the template for the
// type if needed.
goType := strings.TrimPrefix(f.GoType, "[]")
fType := ""
switch f.GoType {
switch goType {
case "uint32":
fType = "%s, err := strconv.ParseUint(%s, 10, 32)"
case "uint64":
Expand Down Expand Up @@ -381,6 +385,11 @@ func createDecodeConvertFunc(f Field) (string, bool) {
var {{.LocalName}} *{{.GoType}}
{{.LocalName}} = &{{.GoType}}{}
err = json.Unmarshal([]byte({{.LocalName}}Str), {{.LocalName}})`

errorCheckingTmpl := `
if err != nil {
return nil, errors.Wrapf(err, "couldn't decode {{.LocalName}} from %v", {{.LocalName}}Str)
}`
// All repeated args of any type are represented as slices, and bare
// assignments to a slice accept a slice as the rvalue. As a result,
// LocalName will be declared as a slice, and json.Unmarshal handles
Expand All @@ -389,28 +398,52 @@ err = json.Unmarshal([]byte({{.LocalName}}Str), {{.LocalName}})`
// provided, and if that fails, we try to unmarshal the string
// surrounded by square brackets. If THAT fails, then the string does
// not represent a valid JSON string and an error is returned.

repeatedFieldType := strings.TrimPrefix(f.GoType, "[]")
convertedVar := "converted"
switch repeatedFieldType {
case "uint32", "int32", "float32":
convertedVar = repeatedFieldType + "(converted)"
}
repeatedUnmarshalTmpl := `
var {{.LocalName}} {{.GoType}}
{{- if and (and .IsBaseType .Repeated) (not (Contains .GoType "[]byte"))}}
err = json.Unmarshal([]byte({{.LocalName}}Str), &{{.LocalName}})
if err != nil {
{{.LocalName}}Str = "[" + {{.LocalName}}Str + "]"
}
{{- if and (.IsBaseType) (not (Contains .GoType "[]byte"))}}
if len({{.LocalName}}StrArr) > 1 {
{{- if (Contains .GoType "[]string")}}
{{.LocalName}} = {{.LocalName}}StrArr
{{- else}}
{{.LocalName}} = make({{.GoType}}, len({{.LocalName}}StrArr))
for i, v := range {{.LocalName}}StrArr {
` + fmt.Sprintf(fType, "converted", "v") + errorCheckingTmpl + `
{{.LocalName}}[i] = ` + convertedVar + `
}
{{- end}}
} else {
{{- end}}
err = json.Unmarshal([]byte({{.LocalName}}Str), &{{.LocalName}})`

errorCheckingTmpl := `
if err != nil {
return nil, errors.Wrapf(err, "couldn't decode {{.LocalName}} from %v", {{.LocalName}}Str)
}`
{{- if (Contains .GoType "[]string")}}
{{.LocalName}} = strings.Split({{.LocalName}}Str, ",")
{{- else if and (and .IsBaseType .Repeated) (not (Contains .GoType "[]byte"))}}
err = json.Unmarshal([]byte({{.LocalName}}Str), &{{.LocalName}})
if err != nil {
{{.LocalName}}Str = "[" + {{.LocalName}}Str + "]"
}
err = json.Unmarshal([]byte({{.LocalName}}Str), &{{.LocalName}})
{{- else}}
err = json.Unmarshal([]byte({{.LocalName}}Str), &{{.LocalName}})
{{- end}}
{{- if and (.IsBaseType) (not (Contains .GoType "[]byte"))}}
}
{{- end}}`

var preamble string
var jsonConvTmpl string
if !f.Repeated {
preamble = singleCustomTypeUnmarshalTmpl
jsonConvTmpl = singleCustomTypeUnmarshalTmpl + errorCheckingTmpl
} else {
preamble = repeatedUnmarshalTmpl
jsonConvTmpl = repeatedUnmarshalTmpl
if repeatedFieldType != "string" {
jsonConvTmpl = repeatedUnmarshalTmpl + errorCheckingTmpl
}
}
jsonConvTmpl := preamble + errorCheckingTmpl
code, err := ApplyTemplate("UnmarshalNonBaseType", jsonConvTmpl, f, TemplateFuncs)
if err != nil {
panic(fmt.Sprintf("Couldn't apply template: %v", err))
Expand Down
10 changes: 9 additions & 1 deletion gengokit/httptransport/templates/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,15 @@ var ClientEncodeTemplate = `
_ = tmp
{{- range $field := $binding.Fields }}
{{- if eq $field.Location "query"}}
{{if or (not $field.IsBaseType) $field.Repeated}}
{{if and $field.Repeated $field.IsBaseType}}
{{- if (Contains $field.GoType "[]string")}}
values["{{$field.QueryParamName}}"] = req.{{$field.CamelName}}
{{- else}}
for _, v := range req.{{$field.CamelName}} {
values.Add("{{$field.QueryParamName}}", fmt.Sprint(v))
}
{{- end}}
{{else if or (not $field.IsBaseType) $field.Repeated}}
tmp, err = json.Marshal(req.{{$field.CamelName}})
if err != nil {
return errors.Wrap(err, "failed to marshal req.{{$field.CamelName}}")
Expand Down