Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullable on required lists #173

Open
dmlambea opened this issue Sep 7, 2023 · 6 comments
Open

Nullable on required lists #173

dmlambea opened this issue Sep 7, 2023 · 6 comments

Comments

@dmlambea
Copy link

dmlambea commented Sep 7, 2023

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
Copy link
Member

vearutop commented Sep 7, 2023

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
Copy link
Author

dmlambea commented Sep 7, 2023

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
Copy link

Related #120

@vearutop
Copy link
Member

@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
Copy link

benbender commented Sep 19, 2023

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
Copy link
Member

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants