fix: correctly map request body field by name

Fixes #325

Author: chebykinn

gengokit/httptransport/httptransport.go

@@ -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)
 

gengokit/httptransport/httptransport_test.go

@@ -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";

gengokit/httptransport/templates/server.go

@@ -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")
@@ -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]
@@ -31,6 +38,10 @@ var ServerDecodeTemplate = `
 			}
 		}
 
+		{{if $binding.RequestRootField}}
+		req.{{$binding.RequestRootField.Name}} = &{{$req_field}}
+		{{end}}
+
 		pathParams := mux.Vars(r)
 		_ = pathParams
 

gengokit/httptransport/templates_test.go

@@ -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 {
@@ -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))
+	}
+}

gengokit/httptransport/types.go

@@ -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

gengokit/template/template.go

@@ -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"
 			}
 		}
 	}

svcdef/consolidate_http.go

@@ -124,7 +124,7 @@ service Map {
 		Name     string
 		Location string
 	}{
-		{"A", "body"},
+		{"A", "body_root"},
 		{"AA", "query"},
 		{"C", "query"},
 		{"MapField", "query"},