search

TheThingsIndustries / protoc-gen-validate

protoc plugin to generate polyglot message validators
Apache License 2.0
1 stars 0 forks source link

Add support for `Validate()` signature #2

Closed rvolosatovs closed 5 years ago

rvolosatovs commented 5 years ago

Generated validators should assert Validate() error apart from ValidateFields(...string) error

johanstokking commented 5 years ago

@rvolosatovs how important is this?

rvolosatovs commented 5 years ago

These are occurences in lorawan-stack:

pkg/frequencyplans/frequencyplans.go
384:func (fp FrequencyPlan) Validate() error {

pkg/ttnpb/lorawan.go
613:func (v RxDelay) Validate() error {
654:func (v MACVersion) Validate() error {
713:func (v PHYVersion) Validate() error {

pkg/rpcmiddleware/validator/validator_test.go
47:func (m *msgWithValidate) Validate() error {

RxDelay, MACVersion, PHYVersion values are validated as enums, which should be good enough FrequencyPlan fields are not validated, but I believe we're not actually sending those in proto messages. If validation is required, a simple solution would be to just refactor FrequencyPlan.Validate() error to FrequencyPlan.ValidateFields(...string) error

johanstokking commented 5 years ago

OK. FrequencyPlan is not a proto and it doesn't have fields. I think we can close this then!