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.

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

nethttp.RequestMapping not work for json input data #61

Closed
keenranger opened this issue Mar 5, 2022 · 2 comments
Closed

nethttp.RequestMapping not work for json input data #61

keenranger opened this issue Mar 5, 2022 · 2 comments

Comments

@keenranger
Copy link

keenranger commented Mar 5, 2022

I want to seperate concerns between usecases and transport it, so I tried nethttp.RequestMapping() as shown on README.md.
But I found it doesn't work for json body, and I started debug the package.

package rest has 6 const ParamIn

const (
	// ParamInPath indicates path parameters, such as `/users/{id}`.
	ParamInPath = ParamIn("path")

	// ParamInQuery indicates query parameters, such as `/users?page=10`.
	ParamInQuery = ParamIn("query")

	// ParamInBody indicates body value, such as `{"id": 10}`.
	ParamInBody = ParamIn("body")

	// ParamInFormData indicates body form parameters.
	ParamInFormData = ParamIn("formData")

	// ParamInCookie indicates cookie parameters, which are passed ParamIn the `Cookie` header,
	// such as `Cookie: debug=0; gdpr=2`.
	ParamInCookie = ParamIn("cookie")

	// ParamInHeader indicates header parameters, such as `X-Header: value`.
	ParamInHeader = ParamIn("header")
)

But in RequestMapping

// RequestMapping creates rest.RequestMapping from struct tags.
//
// This can be used to decouple mapping from usecase input with additional struct.
func RequestMapping(v interface{}) func(h *Handler) {
	return func(h *Handler) {
		m := make(rest.RequestMapping)

		for _, in := range []rest.ParamIn{
			rest.ParamInFormData,
			rest.ParamInQuery,
			rest.ParamInHeader,
			rest.ParamInPath,
			rest.ParamInCookie,
		} {
			mm := make(map[string]string)

			refl.WalkTaggedFields(reflect.ValueOf(v), func(v reflect.Value, sf reflect.StructField, tag string) {
				mm[sf.Name] = tag
			}, string(in))

			if len(mm) > 0 {
				m[in] = mm
			}
		}

		if len(m) > 0 {
			h.ReqMapping = m
		}
	}
}

only 5 of them are used without body for json input. I wonder is this intended.

@vearutop
Copy link
Member

vearutop commented Mar 5, 2022

JSON body, as opposed to other parameters, can be a nested structure of arbitrary complexity. This makes custom field mapping more difficult, than flat map of other parameters.

From a technical perspective, json body is decoded with a standard encoding/json API that does not allow custom mapping interception. Other parameters are decoded with a custom library that allows custom mapping during decoding.

Also json field tag does not have such a direct transport belonging as say query, it is a transport-agnostic serialization tag and might be tolerated as a part of usecase/domain code.

These considerations led to a decision to support custom mapping only for non-json parameters.
I think it is worth updating the docs to make this behavior limitations more visible.

Potentially, mapping handling could be changed to support json fields, though I don't have a good idea on how useful that would be and what could be a strategy for a reliable implementation.

@keenranger
Copy link
Author

To be usecase independant of all transport protocol(e.g. grpc) I thought dividing transport layer and service layer is useful. But I also understand it isn't easy mapping json body.
Thank you for answering and closing issue 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants