Goa not enforcing required attribute

[t-hale]

You'll see that I've defined the attribute UnitsGranted, which is an Int64 that is a required attribute in VestingPlanRequest. The Service, Type and generated code definitions look good, but when I go to run the CLI and server, the Required function seems to not be enforcing anything and it's just being passed through as 0 with no input validation. What am I missing here?

Service Definition

var _ = Service("stox", func() {
	Description("The stox service provides advisors with a comprehensive view of a particular stock schedule.")

	Method("plan", func() {
		Payload(VestingPlanRequest)
		Result(VestingPlanResponse)

		HTTP(func() {
			POST("/plan")
		})

		//GRPC(func() {
		//})
	})

	Files("/openapi.json", "./gen/http/openapi.json")
})

Type Definition

var VestingPlanRequest = Type("VestingPlanRequest", func() {
	Attribute("Symbol", String, "stock symbol to retrieve plan for")
	Attribute("UnitsGranted", Int64, "number of stock units granted")
	Attribute("GrantDate", Date, "initial grant date of equities")
	Attribute("VestDate", Date, "date the equities vest completely")
	Attribute("VestFrequency", VestFrequency, func() {
		Description("frequency of vesting schedule (monthly, quarterly, yearly)")
	})
	Required("Symbol", "UnitsGranted", "GrantDate", "VestDate", "VestFrequency")
})

Generated code

// ValidatePlanRequestBody runs the validations defined on PlanRequestBody
func ValidatePlanRequestBody(body *PlanRequestBody) (err error) {
	if body.Symbol == nil {
		err = goa.MergeErrors(err, goa.MissingFieldError("Symbol", "body"))
	}
	if body.UnitsGranted == nil {
		err = goa.MergeErrors(err, goa.MissingFieldError("UnitsGranted", "body"))
	}
	if body.GrantDate == nil {
		err = goa.MergeErrors(err, goa.MissingFieldError("GrantDate", "body"))
	}
	if body.VestDate == nil {
		err = goa.MergeErrors(err, goa.MissingFieldError("VestDate", "body"))
	}
	if body.VestFrequency == nil {
		err = goa.MergeErrors(err, goa.MissingFieldError("VestFrequency", "body"))
	}
	if body.GrantDate != nil {
		err = goa.MergeErrors(err, goa.ValidateFormat("body.GrantDate", *body.GrantDate, goa.FormatDate))
	}
	if body.VestDate != nil {
		err = goa.MergeErrors(err, goa.ValidateFormat("body.VestDate", *body.VestDate, goa.FormatDate))
	}
	if body.VestFrequency != nil {
		if !(*body.VestFrequency == "monthly" || *body.VestFrequency == "quarterly" || *body.VestFrequency == "yearly") {
			err = goa.MergeErrors(err, goa.InvalidEnumValueError("body.VestFrequency", *body.VestFrequency, []any{"monthly", "quarterly", "yearly"}))
		}
	}
	return
}

Running my CLI

❯ bazel run cmd/server-cli -- --url="http://localhost:8000" stox plan --body '{"GrantDate":"2022-09-13","VestDate":"2023-09-13","VestFrequency":"monthly","Symbol":"AAPL"}'
INFO: Analyzed target //cmd/server-cli:server-cli (1 packages loaded, 8 targets configured).
INFO: Found 1 target...
Target //cmd/server-cli:server-cli up-to-date:
  bazel-bin/cmd/server-cli/server-cli_/server-cli
INFO: Elapsed time: 0.458s, Critical Path: 0.03s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/cmd/server-cli/server-cli_/server-cli '--url=http://localhost:8000' stox plan --body '{"GrantDate":"2022-09-13","VestDate":"2023-09-13","VestFrequency":"monthly","Symbol":"AAPL"}'

Logs on the Server side

❯ bazel run cmd/server
INFO: Analyzed target //cmd/server:server (1 packages loaded, 53 targets configured).
INFO: Found 1 target...
Target //cmd/server:server up-to-date:
  bazel-bin/cmd/server/server_/server
INFO: Elapsed time: 0.567s, Critical Path: 0.05s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/cmd/server/server_/server
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:08-04:00","message":"HTTP \"Plan\" mounted on POST /plan"}
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:08-04:00","message":"HTTP \"./gen/http/openapi.json\" mounted on GET /openapi.json"}
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:08-04:00","message":"HTTP server listening on \"localhost:8000\""}
{"level":"info","service":"stoxapi","from":"127.0.0.1","id":"UNdoVYIT","req":"POST /plan","time":"2023-07-20T22:27:13-04:00","message":"HTTP Request"}
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:13-04:00","message":"stox.plan called with &{Symbol:AAPL UnitsGranted:0 GrantDate:2022-09-13 VestDate:2023-09-13 VestFrequency:monthly}"}
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:14-04:00","message":"numEvents : 13"}
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:14-04:00","message":"unitsGrantedPerEvent : 0"}
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:14-04:00","message":"totalUnitsGranted : 0"}
{"level":"info","service":"stoxapi","time":"2023-07-20T22:27:14-04:00","message":"unitsRemaining : 0"}
{"level":"info","service":"stoxapi","bytes":1619,"id":"UNdoVYIT","status":200,"time":"91.13ms","time":"2023-07-20T22:27:14-04:00","message":"HTTP Request"}

[raphael]

I believe what's happening here is that the Goa generated CLI uses the service type to load the JSON passed on the command line. Because the attribute is required the corresponding field does not use a pointer and so defaults to 0. So the server gets an object with the field set to 0, it is not missing and validation does not fail. To convince yourself use another client tool (e.g. httpie) that won't initialize required fields to their zero value if not explicitly defined.

[t-hale]

That was what I was afraid was happening - is there a way to avoid this besides not using goa? I feel like I'm missing something because this makes it seem like having a Required type isn't possible with a goa cli/server combo?

[raphael]

This is just the CLI code behaving that way when loading bodies from the JSON in the terminal - not the underlying server or client packages so any service making use of these packages won't have this issue.

[t-hale]

Understood, but one of the reasons I'd like to use goa is that it will generate both the cli and server for me. Having to write/maintain a CLI separately from the goa-generated one is taking away one of the main selling points for me. I will look into some of the other validation functions to see if I can work around it in the short term, but would love to see if there is a better long term fix.

Similar to #687

[raphael]

Makes sense, I'd be happy to consider / help putting together a PR that improved this. We may need to generate special types for the CLI instead of relying on the existing service level types to perform that kind of validation.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.