swaggest / rest

Web services with OpenAPI and JSON Schema done quick in Go
https://pkg.go.dev/github.com/swaggest/rest
MIT License
362 stars 17 forks source link

nethttp.RequestMapping not work for json input data #61

Closed keenranger closed 2 years ago

keenranger commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 👍