ggicci / httpin

🍡 HTTP Input for Go - HTTP Request from/to Go Struct (Bi-directional Data Binding between Go Struct and http.Request)
https://ggicci.github.io/httpin/
MIT License
316 stars 23 forks source link

Optional body for POST requests #97

Closed vamshiaruru closed 6 months ago

vamshiaruru commented 7 months ago

Hi! I have been using httpin for a while now and it has been an excellent library. However, I recently noticed that one of my post endpoints didn't have any payload defined, and now I need to add an optional json payload to it (optional because it is imperative I don't break backwards compatibility). This is the sample input type I have:

type RegisterAttendance struct {
    Slug string                   `in:"path=slug" validate:"required"`
}

I need to change it to

type RegisterPayload struct{
    VerifiedBy *string                   `json:"verifiedBy"`
}
type RegisterAttendance struct {
    Slug string                   `in:"path=slug" validate:"required"`
    Payload RegisterPayload `in:"body=json"`
}

However doing this means I can no longer make a post call with empty body. I mandatorily have to send {} as the payload. If I don't send any request body, I get the following error:

 failed: execute directive \"body\" with args [json] failed: EOF

We have clients that have been making calls without sending any request body to this endpoint, and I can't break those calls. If I was doing this without HTTPIN, I'd check of EOF while decoding JSON explicitly. Is there a way I can get this working with HTTPIN?

Thanks for the help!

vamshiaruru commented 7 months ago

I tried this

type OptionalJsonDecoder struct{}

func (o *OptionalJsonDecoder) Decode(src io.Reader, dst interface{}) error {
    err := json.NewDecoder(src).Decode(dst)
    if errors.Is(err, io.EOF) {
        return nil
    }
    return err
}

httpin.RegisterBodyDecoder("optionalJson", &OptionalJsonDecoder{}) // in init

but that doesn't seem to be working. I am getting

panic: httpin: unknown body type: "optionaljson"
vamshiaruru commented 7 months ago

never mind, it looks like both the tag and the first argument of RegisterBodyDecoder have to be lowercased. It works this way. Is it the best way?

ggicci commented 6 months ago

Sorry for the late response. I would like to look into this issue when I have time. I will keep it open.

ggicci commented 6 months ago

The body tag is designed to be case-insensitive because I didn't see a value of distinguishing json and JSON. By looking into the code, I believe in the latest version, we don't need to use the lowercase name of body in the tag.

If you register a body serializer by using optionalJson, internally a body format optionaljson is registerred. And in the tags, you can use both optionalJson and optionaljson, or even OptionalJSON, the latest version should not complain about this.

ggicci commented 6 months ago

By inspecting the "EOF issue", IMO, this error is expected for the bulitin json body directive. Because semantically, the body=json directive just indicates we expect the request body to be a valid JSON input. An empty stream is an invalid JSON input, so the EOF error should and must be raised. I don't want to break it.

So I think the optionalJson custom body format fits this case perfectly.