discord-gophers/goapi-gen

Feature Idea: Add automatic decoding of request bodies

tiehm opened this issue · 3 comments

tiehm commented

Right now you have to manually decode every request body on every request handler which uses request bodies. It's almost always pretty much the same stuff like

var newPet NewPet
if err := json.NewDecoder(r.Body).Decode(&newPet); err != nil {
	sendPetstoreError(w, http.StatusBadRequest, "Invalid format for NewPet")
	return
}

This could be circumvented by introducing a feature to automatically generate/decode the bodies in the generated request handler before it is passed to the handler which is written by the developer.

As an example from the petstore example:
We switch from

// ServerInterface represents all server handlers.
type ServerInterface interface {
	// ...
	// Creates a new pet
	// (POST /pets)
	AddPet(w http.ResponseWriter, r *http.Request)
	// ...
}

to

// ServerInterface represents all server handlers.
type ServerInterface interface {
	// ...
	// Creates a new pet
	// (POST /pets)
	AddPet(w http.ResponseWriter, r *http.Request, body NewPet)
	// ...
}

and directly pass the body the handler which then can implement it and directly use it:

func (p *PetStore) AddPet(w http.ResponseWriter, r *http.Request, body NewPet) {
	fmt.Sprintf("name is %s", body.Name)
}

instead of

func (p *PetStore) AddPet(w http.ResponseWriter, r *http.Request) {
	var body NewPet
	if err := json.NewDecoder(r.Body).Decode(&newPet); err != nil {
		sendPetstoreError(w, http.StatusBadRequest, "Invalid format for NewPet")
		return
	}
	fmt.Sprintf("name is %s", body.Name)
}

This could also implement required checks on body fields and error if those fail automatically without checking it everytime you decode the request body.

The issue behind this is obviously that you now have less control over your error handling for this specific case. A solution could be a global optional function which can be changed to spit out dynamic errors if needed. An example could be:

// maybe a bit too long of a function name :)
func dynamicDecodeErrorsOnRequiredFieldFails(route, field string, err error) string {
    return fmt.Sprintf("invalid body: field %s has issue %s", field, err.Error())
}

or anything like that. This way custom errors could be kept while letting the codegen take care of decoding the body.
Another option could be to not pass the body structure itself to the handler but a struct which has the body in it (if it was successfully decoded) and the error if there is any. This way you have full control over everything but this of course adds a bit more code and changes types.

#13 is already being worked on with the same goals as this.

As for my perspective, I think keeping the flexibility of decoding the body to the handler is better (eg OneOf parameters), as some stuff cannot simply be generated, and making everything optional would be a mess.

tiehm commented

I can see the issues which this could cause.

Closing issue as we are handling this behavior with chi/render.