Required controller input validation

[matthewmueller]

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]

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.

[jfmario]

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?

[matthewmueller]

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]

(Linking an existing draft: #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

[FeldrinH]

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.

[matthewmueller]

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.