search

swaggest / rest

Web services with OpenAPI and JSON Schema done quick in Go
https://pkg.go.dev/github.com/swaggest/rest
MIT License
376 stars 17 forks source link

Nullable on required lists #173

Open dmlambea opened 1 year ago

dmlambea commented 1 year ago

Describe the bug When declaring struct with list i.e.

Comments []string `json:"comments" required:"true"`
Keys     []int    `json:"ints" required:"true" minItems:"1"`

final openapi.json has nullable: true on these fields. Using usecase.Interactor it seems that there is no way to indicate those fields as non-nullable.

Expected behavior Required lists should not be nullable.

vearutop commented 1 year ago

Hello, in my understanding required (as defined by JSON schema) describes only the presence of a property, not its value.

So, {"comments":null,"ints":null} is valid against schema {"required":["comments","ints"]}.

And "minItems":1 is also valid against null value, since it is only checked for arrays.

Given many issues around nullability, I think it makes sense to introduce a new field tag nullable:"false" to allow easy control and override. I'll try to work in a few days.

dmlambea commented 1 year ago

Hello @vearutop

I agree with you in the Comments field regarding required. But please note the Keys field in the example is of array type and it is tagged with minItems:"1", so it would be technically incorrect that the field were nullable.

Anyway, having a field tag nullable, as you propose, would allow full control of the definition.

benbender commented 1 year ago

Related #120

vearutop commented 1 year ago

@dmlambea

But please note the Keys field in the example is of array type and it is tagged with minItems:"1", so it would be technically incorrect that the field were nullable.

As per JSON schema semantics, null is valid in such situation, please check an example https://runkit.com/embed/f8duw012nuiv.

Constraint keywords like minItems are only applied when the type of the value matches, but they don't enforce such type on the value.

benbender commented 1 year ago

It depends on the viewpoint. In go-semantics it's a bit odd to accept null as valid outside of a pointer in a struct. In json schema semantics it seems to be correct.

In my world, the ideal solution would be a global config-switch to switch default-behaviours between go and json schema and a additional struct-tag for finer control.

This wouldn't be a breaking change and give everyone the semantics they may expect to not get confused.

vearutop commented 1 year ago

In go-semantics it's a bit odd to accept null as valid outside of a pointer in a struct.

Not to argue with the necessity of flexible nullability behavior, but I think having nulls for maps and slices is within expectations. At least std JSON encoder in Go treats nil maps/slices this way (without having a ptr).