Skip to content

Commit

Permalink
fix: correctly map request body field by name
Browse files Browse the repository at this point in the history
Fixes #325
  • Loading branch information
chebykinn committed Feb 27, 2021
1 parent 07f56d6 commit e0ea1eb
Show file tree
Hide file tree
Showing 8 changed files with 260 additions and 22 deletions.
4 changes: 4 additions & 0 deletions gengokit/httptransport/httptransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ func NewBinding(i int, meth *svcdef.ServiceMethod) *Binding {
newField.IsEnum = field.Type.Enum != nil
newField.ConvertFunc, newField.ConvertFuncNeedsErrorCheck = createDecodeConvertFunc(newField)
newField.TypeConversion = createDecodeTypeConversion(newField)
if newField.Location == "body_root" {
newField.Location = "body"
nBinding.RequestRootField = &newField
}

nBinding.Fields = append(nBinding.Fields, &newField)

Expand Down
91 changes: 91 additions & 0 deletions gengokit/httptransport/httptransport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,97 @@ func init() {
gopath = filepath.SplitList(os.Getenv("GOPATH"))
}

func TestNewMethodWithBody(t *testing.T) {
defStr := `
syntax = "proto3";
// General package
package general;
import "github.com/metaverse/truss/deftree/googlethirdparty/annotations.proto";
message Inner {
string a = 1;
}
message SumRequest {
int64 a = 1;
Inner in = 2;
}
message SumReply {
int64 v = 1;
string err = 2;
}
service SumSvc {
rpc Sum(SumRequest) returns (SumReply) {
option (google.api.http) = {
put: "/sum/{a}"
body: "in"
};
}
}
`
sd, err := svcdef.NewFromString(defStr, gopath)
if err != nil {
t.Fatal(err, "Failed to create a service from the definition string")
}
innerField := Field{
Name: "In",
QueryParamName: "in",
CamelName: "In",
LowCamelName: "in",
LocalName: "InSum",
Location: "body",
GoType: "pb.Inner",
ConvertFunc: "\nvar InSum *pb.Inner\nInSum = &pb.Inner{}\nerr = json.Unmarshal([]byte(InSumStr), InSum)\nif err != nil {\n\treturn nil, errors.Wrapf(err, \"couldn't decode InSum from %v\", InSumStr)\n}",
ConvertFuncNeedsErrorCheck: false,
TypeConversion: "InSum",
IsBaseType: false,
}
binding := &Binding{
Label: "SumZero",
PathTemplate: "/sum/{a}",
BasePath: "/sum/",
Verb: "put",
RequestRootField: &innerField,
Fields: []*Field{
&Field{
Name: "A",
QueryParamName: "a",
CamelName: "A",
LowCamelName: "a",
LocalName: "ASum",
Location: "path",
GoType: "int64",
ConvertFunc: "ASum, err := strconv.ParseInt(ASumStr, 10, 64)",
ConvertFuncNeedsErrorCheck: true,
TypeConversion: "ASum",
IsBaseType: true,
},
&innerField,
},
}

meth := &Method{
Name: "Sum",
RequestType: "SumRequest",
ResponseType: "SumReply",
Bindings: []*Binding{
binding,
},
}
binding.Parent = meth

newMeth := NewMethod(sd.Service.Methods[0])
t.Logf("%v\n", spew.Sdump(sd.Service.Methods[0]))
if got, want := newMeth, meth; !reflect.DeepEqual(got, want) {
diff := gentesthelper.DiffStrings(spew.Sdump(got), spew.Sdump(want))
t.Errorf("got != want; methods differ: %v\n", diff)
}
}

func TestNewMethod(t *testing.T) {
defStr := `
syntax = "proto3";
Expand Down
13 changes: 12 additions & 1 deletion gengokit/httptransport/templates/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@ var ServerDecodeTemplate = `
// body. Primarily useful in a server.
func DecodeHTTP{{$binding.Label}}Request(_ context.Context, r *http.Request) (interface{}, error) {
defer r.Body.Close()
var req pb.{{GoName $binding.Parent.RequestType}}
{{$req_field := "req" -}}
{{if $binding.RequestRootField -}}
{{$req_field = print "req" ($binding.RequestRootField.Name) -}}
var {{$req_field}} {{$binding.RequestRootField.GoType}}
{{end -}}
buf, err := ioutil.ReadAll(r.Body)
if err != nil {
return nil, errors.Wrapf(err, "cannot read body of http request")
Expand All @@ -19,7 +26,7 @@ var ServerDecodeTemplate = `
unmarshaller := jsonpb.Unmarshaler{
AllowUnknownFields: true,
}
if err = unmarshaller.Unmarshal(bytes.NewBuffer(buf), &req); err != nil {
if err = unmarshaller.Unmarshal(bytes.NewBuffer(buf), &{{$req_field}}); err != nil {
const size = 8196
if len(buf) > size {
buf = buf[:size]
Expand All @@ -31,6 +38,10 @@ var ServerDecodeTemplate = `
}
}
{{if $binding.RequestRootField}}
req.{{$binding.RequestRootField.Name}} = &{{$req_field}}
{{end}}
pathParams := mux.Vars(r)
_ = pathParams
Expand Down
131 changes: 131 additions & 0 deletions gengokit/httptransport/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,15 @@ func TestGenServerDecode(t *testing.T) {
if err != nil {
t.Errorf("Failed to generate server decode code: %v", err)
}

desired := `
// DecodeHTTPSumZeroRequest is a transport/http.DecodeRequestFunc that
// decodes a JSON-encoded sum request from the HTTP request
// body. Primarily useful in a server.
func DecodeHTTPSumZeroRequest(_ context.Context, r *http.Request) (interface{}, error) {
defer r.Body.Close()
var req pb.SumRequest
buf, err := ioutil.ReadAll(r.Body)
if err != nil {
Expand Down Expand Up @@ -211,3 +213,132 @@ func DecodeHTTPSumZeroRequest(_ context.Context, r *http.Request) (interface{},
t.Log(gentesthelper.DiffStrings(got, want))
}
}

func TestGenServerDecodeWithBody(t *testing.T) {
innerField := &Field{
Name: "c",
QueryParamName: "c",
CamelName: "C",
LowCamelName: "c",
LocalName: "CSum",
Location: "body",
GoType: "pb.Inner",
ConvertFunc: "",
ConvertFuncNeedsErrorCheck: true,
TypeConversion: "CSum",
IsBaseType: true,
}
binding := &Binding{
Label: "SumZero",
PathTemplate: "/sum/{a}",
BasePath: "/sum/",
Verb: "get",
RequestRootField: innerField,
Fields: []*Field{
&Field{
Name: "a",
QueryParamName: "a",
CamelName: "A",
LowCamelName: "a",
LocalName: "ASum",
Location: "path",
GoType: "int64",
ConvertFunc: "ASum, err := strconv.ParseInt(ASumStr, 10, 64)",
ConvertFuncNeedsErrorCheck: true,
TypeConversion: "ASum",
IsBaseType: true,
},
&Field{
Name: "b",
QueryParamName: "b",
CamelName: "B",
LowCamelName: "b",
LocalName: "BSum",
Location: "query",
GoType: "int64",
ConvertFunc: "BSum, err := strconv.ParseInt(BSumStr, 10, 64)",
ConvertFuncNeedsErrorCheck: true,
TypeConversion: "BSum",
IsBaseType: true,
},
innerField,
},
}
meth := &Method{
Name: "Sum",
RequestType: "SumRequest",
ResponseType: "SumReply",
Bindings: []*Binding{
binding,
},
}
binding.Parent = meth

str, err := binding.GenServerDecode()
if err != nil {
t.Errorf("Failed to generate server decode code: %v", err)
}
desired := `
// DecodeHTTPSumZeroRequest is a transport/http.DecodeRequestFunc that
// decodes a JSON-encoded sum request from the HTTP request
// body. Primarily useful in a server.
func DecodeHTTPSumZeroRequest(_ context.Context, r *http.Request) (interface{}, error) {
defer r.Body.Close()
var req pb.SumRequest
var reqc pb.Inner
buf, err := ioutil.ReadAll(r.Body)
if err != nil {
return nil, errors.Wrapf(err, "cannot read body of http request")
}
if len(buf) > 0 {
// AllowUnknownFields stops the unmarshaler from failing if the JSON contains unknown fields.
unmarshaller := jsonpb.Unmarshaler{
AllowUnknownFields: true,
}
if err = unmarshaller.Unmarshal(bytes.NewBuffer(buf), &reqc); err != nil {
const size = 8196
if len(buf) > size {
buf = buf[:size]
}
return nil, httpError{errors.Wrapf(err, "request body '%s': cannot parse non-json request body", buf),
http.StatusBadRequest,
nil,
}
}
}
req.c = &reqc
pathParams := mux.Vars(r)
_ = pathParams
queryParams := r.URL.Query()
_ = queryParams
ASumStr := pathParams["a"]
ASum, err := strconv.ParseInt(ASumStr, 10, 64)
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("Error while extracting ASum from path, pathParams: %v", pathParams))
}
req.A = ASum
if BSumStrArr, ok := queryParams["b"]; ok {
BSumStr := BSumStrArr[0]
BSum, err := strconv.ParseInt(BSumStr, 10, 64)
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("Error while extracting BSum from query, queryParams: %v", queryParams))
}
req.B = BSum
}
return &req, err
}
`
if got, want := strings.TrimSpace(str), strings.TrimSpace(desired); got != want {
t.Errorf("Generated code differs from result.\ngot = %s\nwant = %s", got, want)
t.Log(gentesthelper.DiffStrings(got, want))
}
}
1 change: 1 addition & 0 deletions gengokit/httptransport/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Binding struct {
Verb string
Fields []*Field
OneofFields []*OneofField
RequestRootField *Field
// A pointer back to the parent method of this binding. Used within some
// binding methods
Parent *Method
Expand Down
36 changes: 18 additions & 18 deletions gengokit/template/template.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions svcdef/consolidate_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,11 @@ func paramLocation(field *Field, binding *svcparse.HTTPBinding) string {
if optField.Value == "*" {
return "body"
} else if optField.Value == field.Name {
return "body"
return "body_root"
// Have to CamelCase the fields from the protobuf file, as they may
// be lowercase while the name from the Go file will be CamelCased.
} else if gogen.CamelCase(strings.Split(optField.Value, ".")[0]) == field.Name {
return "body"
return "body_root"
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion svcdef/consolidate_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ service Map {
Name string
Location string
}{
{"A", "body"},
{"A", "body_root"},
{"AA", "query"},
{"C", "query"},
{"MapField", "query"},
Expand Down

0 comments on commit e0ea1eb

Please sign in to comment.