Add minimum and maximum validations #10
Conversation
|
Still needed to test some edge cases like float precisions and integer validations. Too late today >.< |
pkg/schemas/model.go
Outdated
| @@ -73,9 +73,9 @@ type Type struct { | |||
| Ref string `json:"$ref,omitempty"` // section 7 | |||
| // RFC draft-wright-json-schema-validation-00, section 5 | |||
| MultipleOf int `json:"multipleOf,omitempty"` // section 5.1 | |||
| Maximum int `json:"maximum,omitempty"` // section 5.2 | |||
| Maximum float64 `json:"maximum,omitempty"` // section 5.2 | |||
There was a problem hiding this comment.
From the specification itself... http://json-schema.org/draft/2019-09/json-schema-validation.html#rfc.section.6.2.2 (and same for minimum)
There was a problem hiding this comment.
btw, all section numbers are wrong
There was a problem hiding this comment.
All the section numbers refer to the quoted draft. We should sync with the latest draft.
There was a problem hiding this comment.
I could do it, but probably in another PR. This one is focusing on the validations feature. Blocked by this weird bug which I don't know how to solve/avoid
|
There are two different things to note at that point: |
jmaeso
changed the title
|
Now there is just the issue with the |
jmaeso
changed the title
|
@jmaeso I'm interested in merging this if you could maybe rebase it from master? |
|
I was testing after applying the rebase and got some issues. When reviewing the errors actually realized that master tests are not passing for me (suffix In case I am the only one experiencing the issue, I'm running |
jmaeso
force-pushed
the
feature/minimum_maximum
branch
from
07fb609
to
cdc97aa
Compare
jmaeso
force-pushed
the
feature/minimum_maximum
branch
from
cdc97aa
to
72aec08
Compare
|
Implementation updated @atombender . Actually looks like I didn't need the ps: feel free to require a different implementation strategy |
|
Oh, incidentally I happened to hack on primitive validation too last night without noticing this, my approach is in #21 (only maximum/exclusiveMaximum done for now, but should be mostly trivial to add the rest). No personal preferences as to which approach to adopt as long as one is merged :) Will leave a few comments here. |
| @@ -642,6 +653,8 @@ func (g *schemaGenerator) generateStructType( | |||
| return nil, fmt.Errorf("could not generate type for field %q: %s", name, err) | |||
| } | |||
|
|
|||
| g.setNumericValidations(prop, &structField) | |||
There was a problem hiding this comment.
If I read this correctly, we add numeric validators if they're present in the schema, even if they're incorrectly set on non-numeric types, which I suppose will cause compile failures. #21 has some measures to protect against this (ignores them).
| } | ||
|
|
||
| func (v *numericValidator) generateMultipleOf(out *codegen.Emitter) { | ||
| out.Println(`if math.Mod(%s.%s, %f) != 0 {`, varNamePlainStruct, v.fieldName, *v.multipleOf) |
There was a problem hiding this comment.
IIUC non-required fields are generated as pointers and this would not handle them properly (would e.g. Mod(*float64,float64), and no nil pointer protection). But glad to be proven wrong if that's the case! Likewise for all other generate* added below. I think the approach in #21 addresses this issue.
| ExclusiveMinimum bool `json:"exclusiveMinimum,omitempty"` // section 5.5 | ||
| MultipleOf *float64 `json:"multipleOf,omitempty"` // section 5.1 | ||
| Maximum *float64 `json:"maximum,omitempty"` // section 5.2 | ||
| ExclusiveMaximum *float64 `json:"exclusiveMaximum,omitempty"` // section 5.3 |
There was a problem hiding this comment.
Changing Exclusive* to floats here means dropping support for draft 4 in this respect. No objections as long as it's a conscious choice. Maybe someone can figure out a clean (enough) way to support both 4 and newer? Union types would help.
|
I am closing this PR as it's become stale, and we'll likely review the whole validation system in the future. |
No description provided.