bud icon indicating copy to clipboard operation
bud copied to clipboard

Required controller input validation

Open matthewmueller opened this issue 2 years ago • 6 comments

Given the following controller/controller.go:

package controller

type Controller struct {}

type User struct {
  Name string `json:"name"`
  Email string `json:"email"`
}

func (c *Controller) Create(name, email string) (*User, error) {
  return &User{name, email}, nil
}

If you run the following request: GET /?name=Alice, you'll get back the following response:

{
  "name": "Alice",
  "email": ""
}

It should actually be a validation error, "email cannot be blank", never run the controller action and most likely return a 422.

matthewmueller avatar Jun 28 '22 05:06 matthewmueller

I'm thinking of doing an easy version of this first where we generate a struct compatible with https://github.com/go-playground/validator.

Long-run I'd like to generate the unmarshaler (maybe using or inspired by easyjson), but that's going to take some time and I'd like to have the interface working earlier.

matthewmueller avatar Jun 30 '22 05:06 matthewmueller

Users already have to add json:"post_id" and so forth to their input structs, would it be a valid solution to document to users that validator.Struct will be executed against the input so that they can add validate tags to struct fields as needed? Or is this depending too much on a third party API?

jfmario avatar Jul 17 '22 02:07 jfmario

That's even more straightforward solution. I think that'd be a great first step!

My main concern of just allowing an unaltered version of this is around needing to break the API at some point. I'm doubtful all the decisions in validator will be good decisions for Bud.

But maybe that could be a 1.x or 2.x transition. I like the idea of getting all the pieces in place, then refining as we learn more.

matthewmueller avatar Jul 17 '22 02:07 matthewmueller

(Linking an existing draft: https://github.com/livebud/bud/discussions/15)

If we go the validator route, we'll still want to generate required struct tags. For example:

func (c *Controller) Signup(email, password string, age *int) error { ... }

Would correspond to the following generated struct:

type SignupInput struct {
  Email string `json:"email" validate:"required"`
  Password string `json:"password" validate:"required"`
  Age *int `json:"age"`
}

Bud's API contract is required by default rather than optional by default like Go. If you want optional, you use pointers.

The documentation has more details: https://www.notion.so/Hey-Bud-4d81622cc49942f9917c5033e5205c69#f385e0bd96d349f1b78da9fbfc305eb6

matthewmueller avatar Jul 24 '22 21:07 matthewmueller

Using validate:"required" has the problem that this makes the default value invalid, so something like {"email": "", "password": ""} would be considered invalid, which would be problematic if the default value is in fact a valid value. This would be especially problematic for numbers, where the default value is 0.

FeldrinH avatar Aug 27 '22 17:08 FeldrinH

That's a good point @FeldrinH. I think this needs to be thought through a bit further, but the place we'd like to get to is when you have

func Signup(email, password string, age *int)

The following would be allowed:

{ "email": "", "password": "" }

But this would not be:

{ "email": "" }

Because password wasn't defined.

matthewmueller avatar Aug 29 '22 01:08 matthewmueller